RE: [PATCH v3 2/5] selftests/resctrl: Make resctrl_tests run using kselftest framework
From: tan.shaopeng@xxxxxxxxxxx
Date: Tue Mar 01 2022 - 02:53:25 EST
Hi Shuah,
> On 2/25/22 1:02 AM, tan.shaopeng@xxxxxxxxxxx wrote:
> > Hi Shuah,
> >
> >> On 2/22/22 12:55 AM, tan.shaopeng@xxxxxxxxxxx wrote:
> >>> Hi Khan,
> >>>
> >>>> On 2/15/22 7:26 PM, Shaopeng Tan wrote:
> >>>>> In kselftest framework, all tests can be build/run at a time, and
> >>>>> a sub test also can be build/run individually. As follows:
> >>>>> $ make -C tools/testing/selftests run_tests $ make -C
> >>>>> tools/testing/selftests TARGETS=ptrace run_tests
> >>>>>
> >>>>> However, resctrl_tests cannot be run using kselftest framework,
> >>>>> users have to change directory to
> >>>>> tools/testing/selftests/resctrl/, run "make" to build executable
> >>>>> file "resctrl_tests", and run "sudo ./resctrl_tests" to execute the test.
> >>>>>
> >>>>> To build/run resctrl_tests using kselftest framework.
> >>>>> Modify tools/testing/selftests/Makefile and
> >>>>> tools/testing/selftests/resctrl/Makefile.
> >>>>>
> >>>>> Even after this change, users can still build/run resctrl_tests
> >>>>> without using framework as before.
> >>>>>
> >>>>> Signed-off-by: Shaopeng Tan <tan.shaopeng@xxxxxxxxxxxxxx>
> >>>>> ---
> >>>>> Some important feedbacks from v1&v2 are addressed as follows:
> >>>>>
> >>>>> - The changelog mentions that changes were made to the resctrl
> >>>>> selftest Makefile but it does not describe what the change
> >> accomplish
> >>>>> or why they are needed.
> >>>>> => By changing the Makefile, resctrl_tests can use kselftest
> >>>>> framework like other sub tests. I described this in changelog.
> >>>>>
> >>>>> - The changelog did not describe how a user may use the kselftest
> >>>>> framework to run the resctrl tests nor the requested information
> >>>>> on how existing workflows are impacted.
> >>>>> => I described how to build/run resctrl_tests with kselftest
> >> framework,
> >>>>> and described the existing workflows are not impacted that
> >>>>> users
> >> can
> >>>>> build/run resctrl_tests without using kselftest framework
> >>>>> as
> >> before.
> >>>>>
> >>>>> - tools/testing/selftests/resctrl/README should be updated.
> >>>>> => I separate the update of README to a new patch.[patch v3
> >>>>> 3/5]
> >>>>>
> >>>>> - Why is the meaning of "EXTRA_SOURCES" (i.e. what is "extra"?) and
> >>>>> why is "SRCS" no longer sufficient?
> >>>>> => I referred to other Makefiles, and found "SRCS" is better
> >>>>> than "EXTRA_SOURCES". So, I updated it to use "SRCS".
> >>>>>
> >>>>> 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..de26638540ba 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
> >>>>> +SRCS := $(wildcard *.c)
> >>>>>
> >>>>> -$(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): $(SRCS)
> >>>>
> >>>> This patch breaks the test build - the below use-cases fail
> >>>>
> >>>> make kselftest-all TARGETS=resctrl
> >>>> make -C tools/testing/selftests/ TARGETS=resctrl
> >>>>
> >>>> Also a simple make in tools/testing/selftests/resctr
> >>>
> >>> Thanks for your feedbacks.
> >>> I applied these patches to the source below and built resctrl_tests
> >>> successfully using above use-cases on x86/arm machine.
> >>> (1)
> >>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> >>> Tag: v5.16
> >>> (2)
> >>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
> >>> Tag: next-20220217
> >>>
> >>> Could you tell me which kernel source you used to build and what
> >>> error message you got?
> >>>
> >>
> >> I tried this on Linux 5.17-rc4
> >
> > I tried these patches on Linux 5.17-rc4 with gcc version 8.4.1 and
> > resctrl_tests is still built successfully.
> >
> > Could you tell me what error message you got when you built it?
>
> Here it is:
>
> make
> gcc resctrl_tests.o cache.c cat_test.c cmt_test.c fill_buf.c mba_test.c
> mbm_test.c resctrlfs.c resctrl_tests.c resctrl_val.c -o resctrl_tests
> /usr/bin/ld: /tmp/ccoarGr4.o:(.bss+0x0): multiple definition of `is_amd';
> resctrl_tests.o:/linux/linux_5.17/tools/testing/selftests/resctrl/resctrl_tests.c
> :16: first defined here
> /usr/bin/ld: /tmp/ccoarGr4.o: in function `detect_amd':
> resctrl_tests.c:(.text+0x63b): multiple definition of `detect_amd';
> resctrl_tests.o:/linux/linux_5.17/tools/testing/selftests/resctrl/resctrl_tests.c
> :19: first defined here
> /usr/bin/ld: /tmp/ccoarGr4.o: in function `tests_cleanup':
> resctrl_tests.c:(.text+0x780): multiple definition of `tests_cleanup';
> resctrl_tests.o:/linux/linux_5.17/tools/testing/selftests/resctrl/resctrl_tests.c
> :50: first defined here
> /usr/bin/ld: /tmp/ccoarGr4.o: in function `main':
> resctrl_tests.c:(.text+0xadd): multiple definition of `main';
> resctrl_tests.o:/linux/linux_5.17/tools/testing/selftests/resctrl/resctrl_tests.c
> :129: first defined here
> collect2: error: ld returned 1 exit status
> make: *** [<builtin>: resctrl_tests] Error 1
I think resctrl_tests.o is an old file you left before applying this patch.
Please remove it before rebuilding.
Before applying this patch, the resctrl_tests.o file was generated and
saved in the current directory(tools/testing/selftests/resctrl/).
After applying this patch, the resctrl_tests.o file was not generated and
the compilation and linking work is done at one time by gcc.
These errors were caused by the previously left resctrl_tests.o file.
> I have gcc (Ubuntu 11.2.0-7ubuntu2) 11.2.0
>
> Take a look at the changes to
> tools/testing/selftests/resctrl/Makefile
>
> I don't think you need to make the changes you made. I would start small with
> including lib.mk and work from there.
Thanks for your advice.
Only the following codes can build resctrl_tests.
+ TEST_GEN_PROGS := resctrl_tests
+ include ../lib.mk
+ $(OUTPUT)/resctrl_tests: $(wildcard *.c)
I will use these codes in next version(v4).
Best regards,
Tan Shaopeng