linux-kselftest.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] selftests: cgroup: build error multiple outpt files
@ 2021-11-05 16:25 Anders Roxell
  2021-11-05 16:25 ` [PATCH 2/2] selftests: cgroup: use function 'labs()' over 'abs()' Anders Roxell
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Anders Roxell @ 2021-11-05 16:25 UTC (permalink / raw)
  To: shuah, christian
  Cc: nathan, ndesaulniers, linux-kselftest, linux-kernel, llvm,
	Anders Roxell, Arnd Bergmann

When building selftests/cgroup: with clang the following error are seen:

clang -Wall -pthread    test_memcontrol.c cgroup_util.c ../clone3/clone3_selftests.h  -o /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol
clang: error: cannot specify -o when generating multiple output files
make[3]: *** [../lib.mk:146: /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol] Error 1

Rework to add the header files to LOCAL_HDRS before including ../lib.mk,
since the dependency is evaluated in '$(OUTPUT)/%:%.c $(LOCAL_HDRS)' in
file lib.mk.

Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---
 tools/testing/selftests/cgroup/Makefile | 12 +++++++-----
 tools/testing/selftests/lib.mk          |  2 +-
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/cgroup/Makefile b/tools/testing/selftests/cgroup/Makefile
index 59e222460581..745fe25fa0b9 100644
--- a/tools/testing/selftests/cgroup/Makefile
+++ b/tools/testing/selftests/cgroup/Makefile
@@ -11,10 +11,12 @@ TEST_GEN_PROGS += test_core
 TEST_GEN_PROGS += test_freezer
 TEST_GEN_PROGS += test_kill
 
+LOCAL_HDRS += $(selfdir)/clone3/clone3_selftests.h $(selfdir)/pidfd/pidfd.h
+
 include ../lib.mk
 
-$(OUTPUT)/test_memcontrol: cgroup_util.c ../clone3/clone3_selftests.h
-$(OUTPUT)/test_kmem: cgroup_util.c ../clone3/clone3_selftests.h
-$(OUTPUT)/test_core: cgroup_util.c ../clone3/clone3_selftests.h
-$(OUTPUT)/test_freezer: cgroup_util.c ../clone3/clone3_selftests.h
-$(OUTPUT)/test_kill: cgroup_util.c ../clone3/clone3_selftests.h ../pidfd/pidfd.h
+$(OUTPUT)/test_memcontrol: cgroup_util.c
+$(OUTPUT)/test_kmem: cgroup_util.c
+$(OUTPUT)/test_core: cgroup_util.c
+$(OUTPUT)/test_freezer: cgroup_util.c
+$(OUTPUT)/test_kill: cgroup_util.c
diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index fe7ee2b0f29c..a40add31a2e3 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -141,7 +141,7 @@ endif
 # Selftest makefiles can override those targets by setting
 # OVERRIDE_TARGETS = 1.
 ifeq ($(OVERRIDE_TARGETS),)
-LOCAL_HDRS := $(selfdir)/kselftest_harness.h $(selfdir)/kselftest.h
+LOCAL_HDRS += $(selfdir)/kselftest_harness.h $(selfdir)/kselftest.h
 $(OUTPUT)/%:%.c $(LOCAL_HDRS)
 	$(LINK.c) $(filter-out $(LOCAL_HDRS),$^) $(LDLIBS) -o $@
 
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/2] selftests: cgroup: use function 'labs()' over 'abs()'
  2021-11-05 16:25 [PATCH 1/2] selftests: cgroup: build error multiple outpt files Anders Roxell
@ 2021-11-05 16:25 ` Anders Roxell
  2021-11-05 20:15   ` Nick Desaulniers
  2021-11-09 15:02   ` Christian Brauner
  2021-11-05 20:10 ` [PATCH 1/2] selftests: cgroup: build error multiple outpt files Nick Desaulniers
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 11+ messages in thread
From: Anders Roxell @ 2021-11-05 16:25 UTC (permalink / raw)
  To: shuah, christian
  Cc: nathan, ndesaulniers, linux-kselftest, linux-kernel, llvm, Anders Roxell

When building selftests/cgroup with clang, the compiler warn about the
function abs() see below:

In file included from test_memcontrol.c:21:
./cgroup_util.h:16:9: warning: absolute value function 'abs' given an argument of type 'long' but has parameter of type 'int' which may cause truncation of value [-Wabsolute-value]
        return abs(a - b) <= (a + b) / 100 * err;
               ^
./cgroup_util.h:16:9: note: use function 'labs' instead
        return abs(a - b) <= (a + b) / 100 * err;
               ^~~
               labs

The note indicates what to do, Rework to use the function 'labs()'.

Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---
 tools/testing/selftests/cgroup/cgroup_util.h | 2 +-
 tools/testing/selftests/cgroup/test_kmem.c   | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/cgroup/cgroup_util.h b/tools/testing/selftests/cgroup/cgroup_util.h
index 82e59cdf16e7..76b35d9dffb5 100644
--- a/tools/testing/selftests/cgroup/cgroup_util.h
+++ b/tools/testing/selftests/cgroup/cgroup_util.h
@@ -13,7 +13,7 @@
  */
 static inline int values_close(long a, long b, int err)
 {
-	return abs(a - b) <= (a + b) / 100 * err;
+	return labs(a - b) <= (a + b) / 100 * err;
 }
 
 extern int cg_find_unified_root(char *root, size_t len);
diff --git a/tools/testing/selftests/cgroup/test_kmem.c b/tools/testing/selftests/cgroup/test_kmem.c
index 22b31ebb3513..d65bb8fe876a 100644
--- a/tools/testing/selftests/cgroup/test_kmem.c
+++ b/tools/testing/selftests/cgroup/test_kmem.c
@@ -192,7 +192,7 @@ static int test_kmem_memcg_deletion(const char *root)
 		goto cleanup;
 
 	sum = slab + anon + file + kernel_stack + pagetables + percpu + sock;
-	if (abs(sum - current) < MAX_VMSTAT_ERROR) {
+	if (labs(sum - current) < MAX_VMSTAT_ERROR) {
 		ret = KSFT_PASS;
 	} else {
 		printf("memory.current = %ld\n", current);
@@ -383,7 +383,7 @@ static int test_percpu_basic(const char *root)
 	current = cg_read_long(parent, "memory.current");
 	percpu = cg_read_key_long(parent, "memory.stat", "percpu ");
 
-	if (current > 0 && percpu > 0 && abs(current - percpu) <
+	if (current > 0 && percpu > 0 && labs(current - percpu) <
 	    MAX_VMSTAT_ERROR)
 		ret = KSFT_PASS;
 	else
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] selftests: cgroup: build error multiple outpt files
  2021-11-05 16:25 [PATCH 1/2] selftests: cgroup: build error multiple outpt files Anders Roxell
  2021-11-05 16:25 ` [PATCH 2/2] selftests: cgroup: use function 'labs()' over 'abs()' Anders Roxell
@ 2021-11-05 20:10 ` Nick Desaulniers
  2021-11-09 15:01 ` Christian Brauner
  2021-11-20  0:22 ` Shuah Khan
  3 siblings, 0 replies; 11+ messages in thread
From: Nick Desaulniers @ 2021-11-05 20:10 UTC (permalink / raw)
  To: Anders Roxell
  Cc: shuah, christian, nathan, linux-kselftest, linux-kernel, llvm,
	Arnd Bergmann, Andrew Delgadillo

On Fri, Nov 5, 2021 at 9:25 AM Anders Roxell <anders.roxell@linaro.org> wrote:
>
> When building selftests/cgroup: with clang the following error are seen:

Thanks for the patches!

typo in subject/oneline, and the `:` above can be dropped.

Andrew reported similar failures throughout selftests:
https://lore.kernel.org/linux-kselftest/20211005222739.2491124-1-adelg@google.com/

Both patches touch the same part of tools/testing/selftests/lib.mk.
This approach looks cleaner to me, but it should reconcile the changes
to tools/testing/selftests/filesystems/binderfs/Makefile that Andrew
made in the link above.

>
> clang -Wall -pthread    test_memcontrol.c cgroup_util.c ../clone3/clone3_selftests.h  -o /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol
> clang: error: cannot specify -o when generating multiple output files
> make[3]: *** [../lib.mk:146: /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol] Error 1
>
> Rework to add the header files to LOCAL_HDRS before including ../lib.mk,
> since the dependency is evaluated in '$(OUTPUT)/%:%.c $(LOCAL_HDRS)' in
> file lib.mk.
>
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> ---
>  tools/testing/selftests/cgroup/Makefile | 12 +++++++-----
>  tools/testing/selftests/lib.mk          |  2 +-
>  2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/selftests/cgroup/Makefile b/tools/testing/selftests/cgroup/Makefile
> index 59e222460581..745fe25fa0b9 100644
> --- a/tools/testing/selftests/cgroup/Makefile
> +++ b/tools/testing/selftests/cgroup/Makefile
> @@ -11,10 +11,12 @@ TEST_GEN_PROGS += test_core
>  TEST_GEN_PROGS += test_freezer
>  TEST_GEN_PROGS += test_kill
>
> +LOCAL_HDRS += $(selfdir)/clone3/clone3_selftests.h $(selfdir)/pidfd/pidfd.h
> +
>  include ../lib.mk
>
> -$(OUTPUT)/test_memcontrol: cgroup_util.c ../clone3/clone3_selftests.h
> -$(OUTPUT)/test_kmem: cgroup_util.c ../clone3/clone3_selftests.h
> -$(OUTPUT)/test_core: cgroup_util.c ../clone3/clone3_selftests.h
> -$(OUTPUT)/test_freezer: cgroup_util.c ../clone3/clone3_selftests.h
> -$(OUTPUT)/test_kill: cgroup_util.c ../clone3/clone3_selftests.h ../pidfd/pidfd.h
> +$(OUTPUT)/test_memcontrol: cgroup_util.c
> +$(OUTPUT)/test_kmem: cgroup_util.c
> +$(OUTPUT)/test_core: cgroup_util.c
> +$(OUTPUT)/test_freezer: cgroup_util.c
> +$(OUTPUT)/test_kill: cgroup_util.c
> diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
> index fe7ee2b0f29c..a40add31a2e3 100644
> --- a/tools/testing/selftests/lib.mk
> +++ b/tools/testing/selftests/lib.mk
> @@ -141,7 +141,7 @@ endif
>  # Selftest makefiles can override those targets by setting
>  # OVERRIDE_TARGETS = 1.
>  ifeq ($(OVERRIDE_TARGETS),)
> -LOCAL_HDRS := $(selfdir)/kselftest_harness.h $(selfdir)/kselftest.h
> +LOCAL_HDRS += $(selfdir)/kselftest_harness.h $(selfdir)/kselftest.h
>  $(OUTPUT)/%:%.c $(LOCAL_HDRS)
>         $(LINK.c) $(filter-out $(LOCAL_HDRS),$^) $(LDLIBS) -o $@
>
> --
> 2.33.0
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] selftests: cgroup: use function 'labs()' over 'abs()'
  2021-11-05 16:25 ` [PATCH 2/2] selftests: cgroup: use function 'labs()' over 'abs()' Anders Roxell
@ 2021-11-05 20:15   ` Nick Desaulniers
  2021-11-09 15:02   ` Christian Brauner
  1 sibling, 0 replies; 11+ messages in thread
From: Nick Desaulniers @ 2021-11-05 20:15 UTC (permalink / raw)
  To: Anders Roxell
  Cc: shuah, christian, nathan, linux-kselftest, linux-kernel, llvm

On Fri, Nov 5, 2021 at 9:25 AM Anders Roxell <anders.roxell@linaro.org> wrote:
>
> When building selftests/cgroup with clang, the compiler warn about the
> function abs() see below:
>
> In file included from test_memcontrol.c:21:
> ./cgroup_util.h:16:9: warning: absolute value function 'abs' given an argument of type 'long' but has parameter of type 'int' which may cause truncation of value [-Wabsolute-value]
>         return abs(a - b) <= (a + b) / 100 * err;
>                ^
> ./cgroup_util.h:16:9: note: use function 'labs' instead
>         return abs(a - b) <= (a + b) / 100 * err;
>                ^~~
>                labs
>
> The note indicates what to do, Rework to use the function 'labs()'.
>
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>

Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>  tools/testing/selftests/cgroup/cgroup_util.h | 2 +-
>  tools/testing/selftests/cgroup/test_kmem.c   | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/cgroup/cgroup_util.h b/tools/testing/selftests/cgroup/cgroup_util.h
> index 82e59cdf16e7..76b35d9dffb5 100644
> --- a/tools/testing/selftests/cgroup/cgroup_util.h
> +++ b/tools/testing/selftests/cgroup/cgroup_util.h
> @@ -13,7 +13,7 @@
>   */
>  static inline int values_close(long a, long b, int err)
>  {
> -       return abs(a - b) <= (a + b) / 100 * err;
> +       return labs(a - b) <= (a + b) / 100 * err;
>  }
>
>  extern int cg_find_unified_root(char *root, size_t len);
> diff --git a/tools/testing/selftests/cgroup/test_kmem.c b/tools/testing/selftests/cgroup/test_kmem.c
> index 22b31ebb3513..d65bb8fe876a 100644
> --- a/tools/testing/selftests/cgroup/test_kmem.c
> +++ b/tools/testing/selftests/cgroup/test_kmem.c
> @@ -192,7 +192,7 @@ static int test_kmem_memcg_deletion(const char *root)
>                 goto cleanup;
>
>         sum = slab + anon + file + kernel_stack + pagetables + percpu + sock;
> -       if (abs(sum - current) < MAX_VMSTAT_ERROR) {
> +       if (labs(sum - current) < MAX_VMSTAT_ERROR) {
>                 ret = KSFT_PASS;
>         } else {
>                 printf("memory.current = %ld\n", current);
> @@ -383,7 +383,7 @@ static int test_percpu_basic(const char *root)
>         current = cg_read_long(parent, "memory.current");
>         percpu = cg_read_key_long(parent, "memory.stat", "percpu ");
>
> -       if (current > 0 && percpu > 0 && abs(current - percpu) <
> +       if (current > 0 && percpu > 0 && labs(current - percpu) <
>             MAX_VMSTAT_ERROR)
>                 ret = KSFT_PASS;
>         else
> --
> 2.33.0
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] selftests: cgroup: build error multiple outpt files
  2021-11-05 16:25 [PATCH 1/2] selftests: cgroup: build error multiple outpt files Anders Roxell
  2021-11-05 16:25 ` [PATCH 2/2] selftests: cgroup: use function 'labs()' over 'abs()' Anders Roxell
  2021-11-05 20:10 ` [PATCH 1/2] selftests: cgroup: build error multiple outpt files Nick Desaulniers
@ 2021-11-09 15:01 ` Christian Brauner
  2021-11-20  0:22 ` Shuah Khan
  3 siblings, 0 replies; 11+ messages in thread
From: Christian Brauner @ 2021-11-09 15:01 UTC (permalink / raw)
  To: Anders Roxell
  Cc: shuah, christian, nathan, ndesaulniers, linux-kselftest,
	linux-kernel, llvm, Arnd Bergmann

On Fri, Nov 05, 2021 at 05:25:29PM +0100, Anders Roxell wrote:
> When building selftests/cgroup: with clang the following error are seen:
> 
> clang -Wall -pthread    test_memcontrol.c cgroup_util.c ../clone3/clone3_selftests.h  -o /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol
> clang: error: cannot specify -o when generating multiple output files
> make[3]: *** [../lib.mk:146: /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol] Error 1
> 
> Rework to add the header files to LOCAL_HDRS before including ../lib.mk,
> since the dependency is evaluated in '$(OUTPUT)/%:%.c $(LOCAL_HDRS)' in
> file lib.mk.
> 
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> ---

Thanks!
Acked-by: Christian Brauner <christian.brauner@ubuntu.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] selftests: cgroup: use function 'labs()' over 'abs()'
  2021-11-05 16:25 ` [PATCH 2/2] selftests: cgroup: use function 'labs()' over 'abs()' Anders Roxell
  2021-11-05 20:15   ` Nick Desaulniers
@ 2021-11-09 15:02   ` Christian Brauner
  1 sibling, 0 replies; 11+ messages in thread
From: Christian Brauner @ 2021-11-09 15:02 UTC (permalink / raw)
  To: Anders Roxell
  Cc: shuah, christian, nathan, ndesaulniers, linux-kselftest,
	linux-kernel, llvm

On Fri, Nov 05, 2021 at 05:25:30PM +0100, Anders Roxell wrote:
> When building selftests/cgroup with clang, the compiler warn about the
> function abs() see below:
> 
> In file included from test_memcontrol.c:21:
> ./cgroup_util.h:16:9: warning: absolute value function 'abs' given an argument of type 'long' but has parameter of type 'int' which may cause truncation of value [-Wabsolute-value]
>         return abs(a - b) <= (a + b) / 100 * err;
>                ^
> ./cgroup_util.h:16:9: note: use function 'labs' instead
>         return abs(a - b) <= (a + b) / 100 * err;
>                ^~~
>                labs
> 
> The note indicates what to do, Rework to use the function 'labs()'.
> 
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> ---

Thanks!
Acked-by: Christian Brauner <christian.brauner@ubuntu.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] selftests: cgroup: build error multiple outpt files
  2021-11-05 16:25 [PATCH 1/2] selftests: cgroup: build error multiple outpt files Anders Roxell
                   ` (2 preceding siblings ...)
  2021-11-09 15:01 ` Christian Brauner
@ 2021-11-20  0:22 ` Shuah Khan
  2021-11-23 14:26   ` Christian Brauner
  3 siblings, 1 reply; 11+ messages in thread
From: Shuah Khan @ 2021-11-20  0:22 UTC (permalink / raw)
  To: Anders Roxell, shuah, christian
  Cc: nathan, ndesaulniers, linux-kselftest, linux-kernel, llvm,
	Arnd Bergmann, Shuah Khan

On 11/5/21 10:25 AM, Anders Roxell wrote:
> When building selftests/cgroup: with clang the following error are seen:
> 
> clang -Wall -pthread    test_memcontrol.c cgroup_util.c ../clone3/clone3_selftests.h  -o /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol
> clang: error: cannot specify -o when generating multiple output files
> make[3]: *** [../lib.mk:146: /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol] Error 1
> 
> Rework to add the header files to LOCAL_HDRS before including ../lib.mk,
> since the dependency is evaluated in '$(OUTPUT)/%:%.c $(LOCAL_HDRS)' in
> file lib.mk.
> 
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> ---
>   tools/testing/selftests/cgroup/Makefile | 12 +++++++-----
>   tools/testing/selftests/lib.mk          |  2 +-
>   2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/testing/selftests/cgroup/Makefile b/tools/testing/selftests/cgroup/Makefile
> index 59e222460581..745fe25fa0b9 100644
> --- a/tools/testing/selftests/cgroup/Makefile
> +++ b/tools/testing/selftests/cgroup/Makefile
> @@ -11,10 +11,12 @@ TEST_GEN_PROGS += test_core
>   TEST_GEN_PROGS += test_freezer
>   TEST_GEN_PROGS += test_kill
>   
> +LOCAL_HDRS += $(selfdir)/clone3/clone3_selftests.h $(selfdir)/pidfd/pidfd.h
> +

This looks odd to me. Why are we introducing dependencies between tests?
clone3 includes in cgroup? Looks odd to me.

thanks,
-- Shuah

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] selftests: cgroup: build error multiple outpt files
  2021-11-20  0:22 ` Shuah Khan
@ 2021-11-23 14:26   ` Christian Brauner
  2021-11-30 16:41     ` Shuah Khan
  0 siblings, 1 reply; 11+ messages in thread
From: Christian Brauner @ 2021-11-23 14:26 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Anders Roxell, shuah, christian, nathan, ndesaulniers,
	linux-kselftest, linux-kernel, llvm, Arnd Bergmann

On Fri, Nov 19, 2021 at 05:22:20PM -0700, Shuah Khan wrote:
> On 11/5/21 10:25 AM, Anders Roxell wrote:
> > When building selftests/cgroup: with clang the following error are seen:
> > 
> > clang -Wall -pthread    test_memcontrol.c cgroup_util.c ../clone3/clone3_selftests.h  -o /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol
> > clang: error: cannot specify -o when generating multiple output files
> > make[3]: *** [../lib.mk:146: /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol] Error 1
> > 
> > Rework to add the header files to LOCAL_HDRS before including ../lib.mk,
> > since the dependency is evaluated in '$(OUTPUT)/%:%.c $(LOCAL_HDRS)' in
> > file lib.mk.
> > 
> > Suggested-by: Arnd Bergmann <arnd@arndb.de>
> > Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> > ---
> >   tools/testing/selftests/cgroup/Makefile | 12 +++++++-----
> >   tools/testing/selftests/lib.mk          |  2 +-
> >   2 files changed, 8 insertions(+), 6 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/cgroup/Makefile b/tools/testing/selftests/cgroup/Makefile
> > index 59e222460581..745fe25fa0b9 100644
> > --- a/tools/testing/selftests/cgroup/Makefile
> > +++ b/tools/testing/selftests/cgroup/Makefile
> > @@ -11,10 +11,12 @@ TEST_GEN_PROGS += test_core
> >   TEST_GEN_PROGS += test_freezer
> >   TEST_GEN_PROGS += test_kill
> > +LOCAL_HDRS += $(selfdir)/clone3/clone3_selftests.h $(selfdir)/pidfd/pidfd.h
> > +
> 
> This looks odd to me. Why are we introducing dependencies between tests?
> clone3 includes in cgroup? Looks odd to me.

The cgroup tests need access to clone3() functionality in order to test
CLONE_INTO_CGROUP which is more suited to be placed alongside the cgroup
tests. There are a few other tests that include the clone3 header.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] selftests: cgroup: build error multiple outpt files
  2021-11-23 14:26   ` Christian Brauner
@ 2021-11-30 16:41     ` Shuah Khan
  2021-12-01 13:06       ` Christian Brauner
  0 siblings, 1 reply; 11+ messages in thread
From: Shuah Khan @ 2021-11-30 16:41 UTC (permalink / raw)
  To: Christian Brauner
  Cc: Anders Roxell, shuah, christian, nathan, ndesaulniers,
	linux-kselftest, linux-kernel, llvm, Arnd Bergmann, Shuah Khan

On 11/23/21 7:26 AM, Christian Brauner wrote:
> On Fri, Nov 19, 2021 at 05:22:20PM -0700, Shuah Khan wrote:
>> On 11/5/21 10:25 AM, Anders Roxell wrote:
>>> When building selftests/cgroup: with clang the following error are seen:
>>>
>>> clang -Wall -pthread    test_memcontrol.c cgroup_util.c ../clone3/clone3_selftests.h  -o /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol
>>> clang: error: cannot specify -o when generating multiple output files
>>> make[3]: *** [../lib.mk:146: /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol] Error 1
>>>
>>> Rework to add the header files to LOCAL_HDRS before including ../lib.mk,
>>> since the dependency is evaluated in '$(OUTPUT)/%:%.c $(LOCAL_HDRS)' in
>>> file lib.mk.
>>>
>>> Suggested-by: Arnd Bergmann <arnd@arndb.de>
>>> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
>>> ---
>>>    tools/testing/selftests/cgroup/Makefile | 12 +++++++-----
>>>    tools/testing/selftests/lib.mk          |  2 +-
>>>    2 files changed, 8 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/cgroup/Makefile b/tools/testing/selftests/cgroup/Makefile
>>> index 59e222460581..745fe25fa0b9 100644
>>> --- a/tools/testing/selftests/cgroup/Makefile
>>> +++ b/tools/testing/selftests/cgroup/Makefile
>>> @@ -11,10 +11,12 @@ TEST_GEN_PROGS += test_core
>>>    TEST_GEN_PROGS += test_freezer
>>>    TEST_GEN_PROGS += test_kill
>>> +LOCAL_HDRS += $(selfdir)/clone3/clone3_selftests.h $(selfdir)/pidfd/pidfd.h
>>> +
>>
>> This looks odd to me. Why are we introducing dependencies between tests?
>> clone3 includes in cgroup? Looks odd to me.
> 
> The cgroup tests need access to clone3() functionality in order to test
> CLONE_INTO_CGROUP which is more suited to be placed alongside the cgroup
> tests. There are a few other tests that include the clone3 header.
> 

If other tests are also including this header, we could move it up under
selftests level. Might have to add include directory.

thanks,
-- Shuah

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] selftests: cgroup: build error multiple outpt files
  2021-11-30 16:41     ` Shuah Khan
@ 2021-12-01 13:06       ` Christian Brauner
  2021-12-03 17:08         ` Shuah Khan
  0 siblings, 1 reply; 11+ messages in thread
From: Christian Brauner @ 2021-12-01 13:06 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Anders Roxell, shuah, christian, nathan, ndesaulniers,
	linux-kselftest, linux-kernel, llvm, Arnd Bergmann

On Tue, Nov 30, 2021 at 09:41:49AM -0700, Shuah Khan wrote:
> On 11/23/21 7:26 AM, Christian Brauner wrote:
> > On Fri, Nov 19, 2021 at 05:22:20PM -0700, Shuah Khan wrote:
> > > On 11/5/21 10:25 AM, Anders Roxell wrote:
> > > > When building selftests/cgroup: with clang the following error are seen:
> > > > 
> > > > clang -Wall -pthread    test_memcontrol.c cgroup_util.c ../clone3/clone3_selftests.h  -o /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol
> > > > clang: error: cannot specify -o when generating multiple output files
> > > > make[3]: *** [../lib.mk:146: /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol] Error 1
> > > > 
> > > > Rework to add the header files to LOCAL_HDRS before including ../lib.mk,
> > > > since the dependency is evaluated in '$(OUTPUT)/%:%.c $(LOCAL_HDRS)' in
> > > > file lib.mk.
> > > > 
> > > > Suggested-by: Arnd Bergmann <arnd@arndb.de>
> > > > Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> > > > ---
> > > >    tools/testing/selftests/cgroup/Makefile | 12 +++++++-----
> > > >    tools/testing/selftests/lib.mk          |  2 +-
> > > >    2 files changed, 8 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/tools/testing/selftests/cgroup/Makefile b/tools/testing/selftests/cgroup/Makefile
> > > > index 59e222460581..745fe25fa0b9 100644
> > > > --- a/tools/testing/selftests/cgroup/Makefile
> > > > +++ b/tools/testing/selftests/cgroup/Makefile
> > > > @@ -11,10 +11,12 @@ TEST_GEN_PROGS += test_core
> > > >    TEST_GEN_PROGS += test_freezer
> > > >    TEST_GEN_PROGS += test_kill
> > > > +LOCAL_HDRS += $(selfdir)/clone3/clone3_selftests.h $(selfdir)/pidfd/pidfd.h
> > > > +
> > > 
> > > This looks odd to me. Why are we introducing dependencies between tests?
> > > clone3 includes in cgroup? Looks odd to me.
> > 
> > The cgroup tests need access to clone3() functionality in order to test
> > CLONE_INTO_CGROUP which is more suited to be placed alongside the cgroup
> > tests. There are a few other tests that include the clone3 header.
> > 
> 
> If other tests are also including this header, we could move it up under
> selftests level. Might have to add include directory.

No objection from me if that's useful. I won't have time for that in the
near future. (This might be of interest for one of the LF programs that
help get new folks interested in kernel development started.)

Christian

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] selftests: cgroup: build error multiple outpt files
  2021-12-01 13:06       ` Christian Brauner
@ 2021-12-03 17:08         ` Shuah Khan
  0 siblings, 0 replies; 11+ messages in thread
From: Shuah Khan @ 2021-12-03 17:08 UTC (permalink / raw)
  To: Christian Brauner
  Cc: Anders Roxell, shuah, christian, nathan, ndesaulniers,
	linux-kselftest, linux-kernel, llvm, Arnd Bergmann, Shuah Khan

On 12/1/21 6:06 AM, Christian Brauner wrote:
> On Tue, Nov 30, 2021 at 09:41:49AM -0700, Shuah Khan wrote:
>> On 11/23/21 7:26 AM, Christian Brauner wrote:
>>> On Fri, Nov 19, 2021 at 05:22:20PM -0700, Shuah Khan wrote:
>>>> On 11/5/21 10:25 AM, Anders Roxell wrote:
>>>>> When building selftests/cgroup: with clang the following error are seen:
>>>>>
>>>>> clang -Wall -pthread    test_memcontrol.c cgroup_util.c ../clone3/clone3_selftests.h  -o /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol
>>>>> clang: error: cannot specify -o when generating multiple output files
>>>>> make[3]: *** [../lib.mk:146: /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol] Error 1
>>>>>
>>>>> Rework to add the header files to LOCAL_HDRS before including ../lib.mk,
>>>>> since the dependency is evaluated in '$(OUTPUT)/%:%.c $(LOCAL_HDRS)' in
>>>>> file lib.mk.
>>>>>
>>>>> Suggested-by: Arnd Bergmann <arnd@arndb.de>
>>>>> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
>>>>> ---
>>>>>     tools/testing/selftests/cgroup/Makefile | 12 +++++++-----
>>>>>     tools/testing/selftests/lib.mk          |  2 +-
>>>>>     2 files changed, 8 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/tools/testing/selftests/cgroup/Makefile b/tools/testing/selftests/cgroup/Makefile
>>>>> index 59e222460581..745fe25fa0b9 100644
>>>>> --- a/tools/testing/selftests/cgroup/Makefile
>>>>> +++ b/tools/testing/selftests/cgroup/Makefile
>>>>> @@ -11,10 +11,12 @@ TEST_GEN_PROGS += test_core
>>>>>     TEST_GEN_PROGS += test_freezer
>>>>>     TEST_GEN_PROGS += test_kill
>>>>> +LOCAL_HDRS += $(selfdir)/clone3/clone3_selftests.h $(selfdir)/pidfd/pidfd.h
>>>>> +
>>>>
>>>> This looks odd to me. Why are we introducing dependencies between tests?
>>>> clone3 includes in cgroup? Looks odd to me.
>>>
>>> The cgroup tests need access to clone3() functionality in order to test
>>> CLONE_INTO_CGROUP which is more suited to be placed alongside the cgroup
>>> tests. There are a few other tests that include the clone3 header.
>>>
>>
>> If other tests are also including this header, we could move it up under
>> selftests level. Might have to add include directory.
> 
> No objection from me if that's useful. I won't have time for that in the
> near future. (This might be of interest for one of the LF programs that
> help get new folks interested in kernel development started.)
> 
> Christian
> 

Thanks. I will apply this fix for now.

thanks,
-- Shuah

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-12-03 17:08 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-05 16:25 [PATCH 1/2] selftests: cgroup: build error multiple outpt files Anders Roxell
2021-11-05 16:25 ` [PATCH 2/2] selftests: cgroup: use function 'labs()' over 'abs()' Anders Roxell
2021-11-05 20:15   ` Nick Desaulniers
2021-11-09 15:02   ` Christian Brauner
2021-11-05 20:10 ` [PATCH 1/2] selftests: cgroup: build error multiple outpt files Nick Desaulniers
2021-11-09 15:01 ` Christian Brauner
2021-11-20  0:22 ` Shuah Khan
2021-11-23 14:26   ` Christian Brauner
2021-11-30 16:41     ` Shuah Khan
2021-12-01 13:06       ` Christian Brauner
2021-12-03 17:08         ` Shuah Khan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).