build.py: build/install kselftests #339

khilman · 2020-03-12T23:31:48Z

Add conditional step to build_kernel that builds the in-kernel
selftests. Build is conditioned on "kselftest" being in
defconfig_extras.

If kselftests are build, install_kernel will create a
kselftests.tar.xz archive that will be published in the publish_step,
and added to bmeta.json (key: kselftests)

Signed-off-by: Kevin Hilman khilman@baylibre.com

khilman · 2020-03-18T18:07:54Z

hmm, I just realized that we don't build the kselftest fragment on staging, so this build will not be tested.

gctucker · 2020-03-25T11:11:04Z

Right, I've just merged the other PR to add kselftest fragments on staging. So this should become active in the next staging run.

gctucker · 2020-03-26T11:51:51Z

OK got some builds, here's one:
https://storage.staging.kernelci.org/kernelci/staging.kernelci.org/staging-20200326.2/arm64/defconfig+kselftest/gcc-8/?C=M&O=D

I haven't checked whether it is as expected, @khilman can you please take a look?

khilman · 2020-03-31T22:47:14Z

The kselftest-install step is failing due to

/bin/sh: 1: rsync: not found

Which is now addressed by #345

Note that this doesn't cause the whole build to fail, just the kselftests to not be installed.

khilman · 2020-05-01T21:41:37Z

@gctucker this one should be ready now, no?

gctucker · 2020-05-04T09:06:15Z

It would be a lot better to keep the kselftest steps in separate functions, the build_kernel() and install_kernel() functions are already doing a lot and they should really be only dealing with kernel builds. Ideally we should have a way to build kselftest with a command line argument. The test for 'kselftest in defconfig_extras' is quite fragile, this logic should be determined at a higher level.

Then I'm not sure of the impact of converting the output path from relative to absolute, there might be some unwanted side effects.

gctucker · 2020-05-22T06:52:54Z

From a functional point of view, it seems to be doing the right thing. Here's a sample build:
https://storage.staging.kernelci.org/kernelci/staging.kernelci.org/staging-20200520.2/arm64/defconfig+kselftest/gcc-8/

which includes a kselftest tarball:
https://storage.staging.kernelci.org/kernelci/staging.kernelci.org/staging-20200520.2/arm64/defconfig+kselftest/gcc-8/kselftest.tar.xz

Taking a quick look, this seems fine:

$ ls
android      ftrace        kselftest  lkdtm           pstore            sysctl  zram
cpufreq      futex         lib        memory-hotplug  run_kselftest.sh  tpm2
cpu-hotplug  intel_pstate  livepatch  net             static_keys       user

and there doesn't seem to be any side-effects of this PR on other "regular" builds.

It would be nice to at least try to run a test job with the contents of this archive to completely verify it was built correctly, maybe with a manually modified kernelci rootfs to skip the actual LAVA integration part.

Then with a bit of code clean-up this should be ready to go. We should probably build a very small set of kernels with kselftest until we can actually run tests, just enough to facilitate things a bit.

gctucker · 2020-05-22T06:56:15Z

kernelci/build.py

+        # Ideally this should just be a 'make kselftest-install', but
+        # due to bugs with O= in kselftest Makefile, this has to be
+        # 'make -C tools/testing/selftests install'


Any chance these bugs can be fixed? Is there any email thread about this topic to follow what is going?

There were some similar discussions, but nothing on this specifially. I'll start an email thread on the kselftest list.

However, even if this gets fixed upstream, it's unlikely this would be backported, so I think we should stick with the workaround in this patch for awhile, especially if we expect to run kselftest on the stable kernels.

FWIW, I think this was fixed in v5.7 (see merge 397a97946798890b9bdaa6122fcfad7147690670), but perhaps I'm not understanding the specific bug as it related to running under kernelci's builder.

@khilman Can you please add a link to the discussion in the kselftest mailing list archives?

There are several related issues around this:

https://lore.kernel.org/lkml/158351957799.3363.15269768530697526765.stgit@devnote2/

https://lore.kernel.org/linux-kselftest/MWHPR13MB0895B92C9B4807D94E1E6B04FDE30@MWHPR13MB0895.namprd13.prod.outlook.com/

https://lore.kernel.org/bpf/cover.1583358715.git.skhan@linuxfoundation.org/

As well as some interesting tools to help sanity check:

https://lore.kernel.org/linux-kselftest/86508e9d-4aff-d9a0-3783-971a4686ccd1@bird.org/

But again, even though most (maybe all?) of these issues are fixed upstream, it's unlikely these all be backported to stable, so the workaround in this series is still necessary if we want to build kselftest on stable.

gctucker · 2020-06-09T21:42:47Z

Let's focus on the k8s stuff merged first and then work on resolving this one. I think it's 90% good, might just need rebasing and a couple of minor tweaks.

khilman · 2020-07-28T17:59:22Z

I wasn't aware of the gen_tar make target in tools/testing/selftest until seeing @kees comment in #331, I've reworked this PR slightly to use that instead of manually creating the tarball in kernelci/build.py.

Also rebased onto current master.

khilman · 2020-07-29T20:17:08Z

Removed a stray debug print.
Rebased to current master.

kees · 2020-07-30T18:23:51Z

kernelci/build.py

@@ -787,6 +807,17 @@ def install_kernel(kdir, tree_name, tree_url, git_branch, git_commit=None,
        shell_cmd("tar -C{path} -cJf {tarball} .".format(
            path=mod_path, tarball=modules_tarball_path))

+    # 'make gen_tar' creates this tarball path
+    kselftest_tarball = 'kselftest-packages/kselftest.tar.gz'


Commit says ".xz", code expects ".gz", but depends on the default behavior of "gen_tar" without a FORMAT= specified. Perhaps explicitly declare "FORMAT=.xz" (or =.gz) (and update the filename and commit message appropriately)?

Right, I'll make it more explicit and clean up the changelog. Thanks for the review.

FWIW, I started with .xz for size reasons, but we're recycling a test-plan from another test project (Linaro LKFT) and their test wrappers etc. have a bunch of hard-coded assumptions for .gz, so I switched to that (but forgot to update commit message.)

Add conditional step to build_kernel that builds the in-kernel selftests. Build is conditioned on "kselftest" being in defconfig_extras. The build step will also create a kselftest tarball using the 'make gen_tar' target in tools/testing/selftests. If kselftests are build, install_kernel will copy the compressed tarball into the install dir, and published in the publish_step, and added to bmeta.json (key: kselftests) NOTE: make gen_tar explicitly uses FORMAT=.gz to generate a gzip'd tarball because the LKFT test-plan we use for kselftest has hard-coded assumptions about the Signed-off-by: Kevin Hilman <khilman@baylibre.com>

khilman · 2020-08-25T20:22:33Z

Updated PR: dropped patch which updated rootfs image since this is now updated in master branch.

gctucker

Overall, I think this is OK to get merged and rolled out in production. A couple of things that should be kept in mind though:

Adding test binaries to a kernel build is not a great precedent, as it blurs the definition of what a kernel build is for KernelCI. Of course, kselftest is a bit special since it's tied to the kernel source. Still, adding other things in the future like documentation or any other artifacts that can be built within a kernel tree is probably going to cause us some problems. So we should try to deal with that by having a separate place for "kernel extras" that aren't actually the kernel, but built from the kernel tree.
Building everything in the build_kernel function is not great, that's not new but it's not going in the right direction with kselftest being added there too. As a result, we can't build a kernel with kselftest config enabled without also building the kselftest tarball. That will need some slight redesign all the way to the kci_build command line interface, so it's probably best to tackle that once the settings file support is merged and when all the data-related operations go via kernelci.data (i.e. publish_kernel, push_kernel etc...).

gctucker · 2020-08-26T08:55:26Z

kernelci/build.py

@@ -467,7 +467,8 @@ def _run_make(kdir, arch, target=None, jopt=None, silent=True, cc='gcc',
        args.append('CC={}'.format(cc))

    if output:
-        args.append('O={}'.format(os.path.relpath(output, kdir)))
+        # due to kselftest Makefile issues, O= cannot be a relative path
+        args.append('O={}'.format(os.path.abspath(output)))


OK tested manually with arbitrary output paths.

gctucker · 2020-08-26T09:17:02Z

Tested OK on staging, combined with #445: https://staging.kernelci.org/test/plan/id/5f45aff802c5b617e7b1458c/

khilman requested a review from gctucker March 12, 2020 23:31

gctucker requested a review from a team March 13, 2020 15:53

gctucker added this to In Progress in KernelCI project board via automation Mar 13, 2020

gctucker reviewed May 22, 2020

View reviewed changes

kees mentioned this pull request Jul 5, 2020

kselftest #331

Closed

5 tasks

khilman linked an issue Jul 22, 2020 that may be closed by this pull request

kselftest #331

Closed

5 tasks

khilman force-pushed the dev/build-kselftests branch from 966dc30 to 3e22353 Compare July 28, 2020 17:57

khilman force-pushed the dev/build-kselftests branch from 3e22353 to 36b6b5e Compare July 29, 2020 20:15

kees reviewed Jul 30, 2020

View reviewed changes

khilman force-pushed the dev/build-kselftests branch from 36b6b5e to d42a6b1 Compare July 31, 2020 17:57

khilman force-pushed the dev/build-kselftests branch from d42a6b1 to 989da10 Compare August 11, 2020 17:06

gctucker approved these changes Aug 26, 2020

View reviewed changes

gctucker merged commit b6518cb into kernelci:master Aug 26, 2020

KernelCI project board automation moved this from In Progress to Done Aug 26, 2020

khilman deleted the dev/build-kselftests branch January 18, 2023 18:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build.py: build/install kselftests #339

build.py: build/install kselftests #339

khilman commented Mar 12, 2020

khilman commented Mar 18, 2020

gctucker commented Mar 25, 2020

gctucker commented Mar 26, 2020

khilman commented Mar 31, 2020 •

edited

Loading

khilman commented May 1, 2020

gctucker commented May 4, 2020

gctucker commented May 22, 2020

gctucker May 22, 2020

khilman Jul 28, 2020

kees Jul 30, 2020

gctucker Aug 11, 2020

khilman Aug 11, 2020

gctucker commented Jun 9, 2020

khilman commented Jul 28, 2020

khilman commented Jul 29, 2020

kees Jul 30, 2020 •

edited

Loading

khilman Jul 30, 2020

khilman commented Aug 25, 2020

gctucker left a comment

gctucker Aug 26, 2020

gctucker commented Aug 26, 2020

build.py: build/install kselftests #339

build.py: build/install kselftests #339

Conversation

khilman commented Mar 12, 2020

khilman commented Mar 18, 2020

gctucker commented Mar 25, 2020

gctucker commented Mar 26, 2020

khilman commented Mar 31, 2020 • edited Loading

khilman commented May 1, 2020

gctucker commented May 4, 2020

gctucker commented May 22, 2020

gctucker May 22, 2020

Choose a reason for hiding this comment

khilman Jul 28, 2020

Choose a reason for hiding this comment

kees Jul 30, 2020

Choose a reason for hiding this comment

gctucker Aug 11, 2020

Choose a reason for hiding this comment

khilman Aug 11, 2020

Choose a reason for hiding this comment

gctucker commented Jun 9, 2020

khilman commented Jul 28, 2020

khilman commented Jul 29, 2020

kees Jul 30, 2020 • edited Loading

Choose a reason for hiding this comment

khilman Jul 30, 2020

Choose a reason for hiding this comment

khilman commented Aug 25, 2020

gctucker left a comment

Choose a reason for hiding this comment

gctucker Aug 26, 2020

Choose a reason for hiding this comment

gctucker commented Aug 26, 2020

khilman commented Mar 31, 2020 •

edited

Loading

kees Jul 30, 2020 •

edited

Loading