Re: [GIT PULL] Kselftest update for Linux 5.2-rc1

[Alexei Starovoitov]

On Thu, May 09, 2019 at 07:42:09PM -0600, Shuah Khan wrote:
> On 5/9/19 4:40 PM, Shuah Khan wrote:
> > On 5/9/19 4:20 PM, Alexei Starovoitov wrote:
> > > On Mon, May 06, 2019 at 10:56:56AM -0600, Shuah Khan wrote:
> > > > Hi Linus,
> > > > 
> > > > Please pull the following Kselftest update for Linux 5.2-rc1
> > > > 
> > > > This Kselftest update for Linux 5.2-rc1 consists of
> > > > 
> > > > - fixes to seccomp test, and kselftest framework
> > > > - cleanups to remove duplicate header defines
> > > > - fixes to efivarfs "make clean" target
> > > > - cgroup cleanup path
> > > > - Moving the IMA kexec_load selftest to selftests/kexec work from
> > > >    Mimi Johar and Petr Vorel
> > > > - A framework to kselftest for writing kernel test modules addition
> > > >    from Tobin C. Harding
> > > > 
> > > > diff is attached.
> > > > 
> > > > thanks,
> > > > -- Shuah
> > > > 
> > > > 
> > > > ----------------------------------------------------------------
> > > > The following changes since commit
> > > > 15ade5d2e7775667cf191cf2f94327a4889f8b9d:
> > > > 
> > > >    Linux 5.1-rc4 (2019-04-07 14:09:59 -1000)
> > > > 
> > > > are available in the Git repository at:
> > > > 
> > > >    git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest
> > > > tags/linux-kselftest-5.2-rc1
> > > > 
> > > > for you to fetch changes up to d917fb876f6eaeeea8a2b620d2a266ce26372f4d:
> > > > 
> > > >    selftests: build and run gpio when output directory is the src dir
> > > > (2019-04-22 17:02:26 -0600)
> > > > 
> > > > ----------------------------------------------------------------
> > > > linux-kselftest-5.2-rc1
> > > > 
> > > > This Kselftest update for Linux 5.2-rc1 consists of
> > > > 
> > > > - fixes to seccomp test, and kselftest framework
> > > > - cleanups to remove duplicate header defines
> > > > - fixes to efivarfs "make clean" target
> > > > - cgroup cleanup path
> > > > - Moving the IMA kexec_load selftest to selftests/kexec work from
> > > >    Mimi Johar and Petr Vorel
> > > > - A framework to kselftest for writing kernel test modules addition
> > > >    from Tobin C. Harding
> > > > 
> > > > ----------------------------------------------------------------
> > > > Kees Cook (3):
> > > >        selftests/seccomp: Handle namespace failures gracefully
> > > >        selftests/harness: Add 30 second timeout per test
> > > >        selftests/ipc: Fix msgque compiler warnings
> > > > 
> > > > Mathieu Desnoyers (1):
> > > >        rseq/selftests: Adapt number of threads to the number of
> > > > detected cpus
> > > > 
> > > > Mimi Zohar (9):
> > > >        selftests/kexec: move the IMA kexec_load selftest to
> > > > selftests/kexec
> > > >        selftests/kexec: cleanup the kexec selftest
> > > >        selftests/kexec: define a set of common functions
> > > >        selftests/kexec: define common logging functions
> > > >        selftests/kexec: define "require_root_privileges"
> > > >        selftests/kexec: kexec_file_load syscall test
> > > >        selftests/kexec: check kexec_load and kexec_file_load are enabled
> > > >        selftests/kexec: make kexec_load test independent of IMA
> > > > being enabled
> > > >        selftests/kexec: update get_secureboot_mode
> > > > 
> > > > Petr Vorel (1):
> > > >        selftests/kexec: Add missing '=y' to config options
> > > > 
> > > > Po-Hsu Lin (1):
> > > >        selftests/efivarfs: clean up test files from test_create*()
> > > > 
> > > > Roman Gushchin (1):
> > > >        selftests: cgroup: fix cleanup path in
> > > > test_memcg_subtree_control()
> > > > 
> > > > Sabyasachi Gupta (4):
> > > >        selftest/x86/mpx-dig.c: Remove duplicate header
> > > >        selftest/timers: Remove duplicate header
> > > >        selftest/rseq: Remove duplicate header
> > > >        selftest/gpio: Remove duplicate header
> > > > 
> > > > Shuah Khan (2):
> > > >        selftests: fix headers_install circular dependency
> > > 
> > > Shuah,
> > > 
> > > the commit 8ce72dc32578 ("selftests: fix headers_install circular
> > > dependency")
> > > broke our build/test workflow, since it added:
> > >    ifneq ($(KBUILD_OUTPUT),)
> > >            OUTPUT := $(KBUILD_OUTPUT)
> > >    else
> > > 
> > > which means that all of selftests/bpf artifacts are now going into
> > > main build directory cluttering it with all sorts of .o, generated files
> > > and executables.
> > > The end result is humans and scripts can no longer find tests.
> 
> bpf build fails with the above commit. However, even without it, I am
> seeing bpf objects going to tools/testing/selftests/bpf
> 
> I reverted the commit and ran your use-case:
> 
> export KBUILD_OUTPUT=/tmp/kselftest_bpf
> cd tools/testing/selftests/bpf/
> make
> ./test_verifier
> 
> I see bpf objects in tools/testing/selftests/bpf/ and I can run the
> test.
> 
> What am I missing? The only way ./test_verifier would work is if
> test_verifier is in tools/testing/selftests/bpf/

That's the point. All artifacts should be in tools/testing/selftests/bpf/
if 'make' was done there regardless of KBUILD_OUTPUT.

> I am curious what you are actually seeing with this commit?
> 
> With the 8ce72dc32578
> 
> What I see is - if KBUILD_OUTPUT directory is missing, then the make
> just fails and the following diff fixes that problem:

KBUILD_OUTPUT is a valid dir where kernel build goes.

> diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
> index 098dd0065fb1..074ce7d26a9d 100644
> --- a/tools/testing/selftests/lib.mk
> +++ b/tools/testing/selftests/lib.mk
> @@ -13,6 +13,7 @@ ifeq (0,$(MAKELEVEL))
>                 DEFAULT_INSTALL_HDR_PATH := 1
>         endif
>      endif
> +$(shell mkdir -p $(OUTPUT))

makefile should not create the dir this automatically.

>  endif
>  selfdir = $(realpath $(dir $(filter %/lib.mk,$(MAKEFILE_LIST))))
> 
> 
> Now when I run
> 
> cd tools/testing/selftests/bpf/
> make
> ./test_verifier
> 
> bpf objects end up in /tmp/kselftest_bpf
> 
> which is what should happen when KBUILD_OUPUT is set.

No. KBUILD_OUPUT must be ignored while building selftests/bpf.
Just like it's ignored when samples/bpf/ are build.
People do 'cd samples/bpf; make; run stuff'
and they do the same 'cd .../selftests/bpf; make; run'