Re: [PATCH v3 2/5] selftests/resctrl: Make resctrl_tests run using kselftest framework

[Shuah Khan <skhan@linuxfoundation.org>]

On 2/25/22 1:02 AM, tan.shaopeng@fujitsu.com wrote:
> Hi Shuah,
> 
>> On 2/22/22 12:55 AM, tan.shaopeng@fujitsu.com 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@jp.fujitsu.com>
>>>>> ---
>>>>> 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 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,
-- Shuah