Re: [PATCH] selftests/vm: fix inability to build any vm tests

[John Hubbard <jhubbard@nvidia.com>]

On 8/17/22 14:13, Axel Rasmussen wrote:
> Fixes: f2745dc0ba3d ("selftests: stop using KSFT_KHDR_INSTALL")
> Signed-off-by: Axel Rasmussen <axelrasmussen@google.com>
> ---
>   tools/testing/selftests/vm/Makefile | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile
> index d9fa6a9ea584..f2a12494f2d8 100644
> --- a/tools/testing/selftests/vm/Makefile
> +++ b/tools/testing/selftests/vm/Makefile
> @@ -1,7 +1,7 @@
>   # SPDX-License-Identifier: GPL-2.0
>   # Makefile for vm selftests
>   
> -LOCAL_HDRS += $(selfdir)/vm/local_config.h $(top_srcdir)/mm/gup_test.h
> +LOCAL_HDRS += $(selfdir)/vm/local_config.h $(selfdir)/../../../mm/gup_test.h

Hi Alex,

Thanks for fixing up this build, it's always frustrating to finally
finish working on something, only to find that the selftests build is
broken!

This looks correct, and also I've tested it locally, and it works. So
please feel free to add:

Reviewed-by: John Hubbard <jhubbard@nvidia.com>


A couple of follow-up thoughts:

1) I recalled that hmm-tests.c in the same directory also needs
gup_test.h, and wondered if that was covered. And it turns out the the
relative "up and over" include path is done in hmm-tests.c itself,
instead of in the Makefile, like this:

/*
  * This is a private UAPI to the kernel test module so it isn't exported
  * in the usual include/uapi/... directory.
  */
#include "../../../../lib/test_hmm_uapi.h"
#include "../../../../mm/gup_test.h"

It would be nice (maybe follow-up polishing for someone) if this were
done once, instead of twice (or more?) via different (source code vs.
Makefile) mechanisms.

2) Commit f2745dc0ba3d ("selftests: stop using KSFT_KHDR_INSTALL")
claims that it is now required to "make headers_install" before building
the selftests. However, after applying your fix (not to imply that there
is anything wrong with the fix; it's fine), I am still able to build
vm/selftests, directly after a top-level "make clean".

I believe that this is because the selftests are directly including
gup_test.h, via relative up-and-over paths. And I recall (and also did a
quick dry run, to be sure) that this internal gup_test.h header is not
part of the headers_install list. So that seems to be all working as
intended. But I wanted to say all of this out loud, in order to be sure
I fully understand these build steps.


thanks,
-- 
John Hubbard
NVIDIA