linux-kernel - Re: [PATCH v2 2/5] selftests/resctrl: Make resctrl

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <dfd74b29-b68e-588a-b8f5-5ca7ff216819@intel.com>
Date:   Thu, 6 Jan 2022 15:48:39 -0800
From:   Reinette Chatre <reinette.chatre@...el.com>
To:     Shaopeng Tan <tan.shaopeng@...fujitsu.com>,
        Fenghua Yu <fenghua.yu@...el.com>,
        Shuah Khan <shuah@...nel.org>
CC:     <linux-kernel@...r.kernel.org>, <linux-kselftest@...r.kernel.org>
Subject: Re: [PATCH v2 2/5] selftests/resctrl: Make resctrl_tests run using
 kselftest framework

Hi Shaopeng Tan,

On 12/13/2021 2:01 AM, Shaopeng Tan wrote:
> This commit enables resctrl_tests to be built/run in kselftest framework.

(This commit)

> Build/run resctrl_tests by building/running all tests of kselftest, or by using
> the "TARGETS" variable on the make command line to specify resctrl_tests.

Please review the feedback I provided to your first version. I do not
see the changelog improvements that describe how a user may use the
kselftest framework to run the resctrl tests nor the requested information
on how existing workflows are impacted. Going back through my previous review
feedback to ensure that all is addressed is very time consuming and causes
considerable delay when needing to review newer versions. 

tools/testing/selftests/resctrl/README should be updated also

> This commit modified the Makefile of kernel kselftest set and
> the Makefile of resctrl_tests.

This is clear from the diffstat. You could elaborate why the changes were
needed instead and do so without the "This commit" usage.

> 
> Signed-off-by: Shaopeng Tan <tan.shaopeng@...fujitsu.com>
> ---
>  tools/testing/selftests/Makefile         |  1 +
>  tools/testing/selftests/resctrl/Makefile | 20 ++++++--------------
>  2 files changed, 7 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index c852eb40c4f7..7df397c6893c 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -51,6 +51,7 @@ TARGETS += proc
>  TARGETS += pstore
>  TARGETS += ptrace
>  TARGETS += openat2
> +TARGETS += resctrl
>  TARGETS += rlimits
>  TARGETS += rseq
>  TARGETS += rtc
> diff --git a/tools/testing/selftests/resctrl/Makefile b/tools/testing/selftests/resctrl/Makefile
> index 6bcee2ec91a9..c9e8540fc594 100644
> --- a/tools/testing/selftests/resctrl/Makefile
> +++ b/tools/testing/selftests/resctrl/Makefile
> @@ -1,17 +1,9 @@
> -CC = $(CROSS_COMPILE)gcc
> -CFLAGS = -g -Wall -O2 -D_FORTIFY_SOURCE=2
> -SRCS=$(wildcard *.c)
> -OBJS=$(SRCS:.c=.o)
> +CFLAGS += -g -Wall -O2 -D_FORTIFY_SOURCE=2
>  
> -all: resctrl_tests
> +TEST_GEN_PROGS := resctrl_tests
> +EXTRA_SOURCES := $(wildcard *.c)

Why is the meaning of "EXTRA_SOURCES" (i.e. what is "extra"?) and
why is "SRCS" no longer sufficient?

>  
> -$(OBJS): $(SRCS)
> -	$(CC) $(CFLAGS) -c $(SRCS)
> +all: $(TEST_GEN_PROGS)
>  
> -resctrl_tests: $(OBJS)
> -	$(CC) $(CFLAGS) -o $@ $^
> -
> -.PHONY: clean
> -
> -clean:
> -	$(RM) $(OBJS) resctrl_tests
> +$(TEST_GEN_PROGS): $(EXTRA_SOURCES)
> +include ../lib.mk

Reinette