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@xxxxxxxxxxxxxx> > --- > 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
