As our set of multiarch tests has grown the practice of running every
plugin with every test is becoming unsustainable. If we switch to
ensuring every test gets run with at least one plugin we can speed
things up.
Some plugins do need to be run with specific tests (for example the
memory instrumentation test). We can handle this by manually adding
them to EXTRA_RUNS. We also need to wrap rules in a CONFIG_PLUGIN test
so we don't enable the runs when plugins are not enabled.
Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-ID: <20250725154517.3523095-12-alex.bennee@linaro.org>
diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target
index a12b15637ea..18afd5be194 100644
--- a/tests/tcg/Makefile.target
+++ b/tests/tcg/Makefile.target
@@ -173,14 +173,25 @@ PLUGINS=$(filter-out $(DISABLE_PLUGINS), \
# We need to ensure expand the run-plugin-TEST-with-PLUGIN
# pre-requistes manually here as we can't use stems to handle it. We
# only expand MULTIARCH_TESTS which are common on most of our targets
-# to avoid an exponential explosion as new tests are added. We also
-# add some special helpers the run-plugin- rules can use below.
+# and rotate the plugins so we don't grow too out of control as new
+# tests are added. Plugins that need to run with a specific test
+# should ensure they add their combination to EXTRA_RUNS.
ifneq ($(MULTIARCH_TESTS),)
-$(foreach p,$(PLUGINS), \
- $(foreach t,$(MULTIARCH_TESTS),\
- $(eval run-plugin-$(t)-with-$(p): $t $p) \
- $(eval RUN_TESTS+=run-plugin-$(t)-with-$(p))))
+
+NUM_PLUGINS := $(words $(PLUGINS))
+NUM_TESTS := $(words $(MULTIARCH_TESTS))
+
+define mod_plus_one
+ $(shell $(PYTHON) -c "print( ($(1) % $(2)) + 1 )")
+endef
+
+$(foreach _idx, $(shell seq 1 $(NUM_TESTS)), \
+ $(eval _test := $(word $(_idx), $(MULTIARCH_TESTS))) \
+ $(eval _plugin := $(word $(call mod_plus_one, $(_idx), $(NUM_PLUGINS)), $(PLUGINS))) \
+ $(eval run-plugin-$(_test)-with-$(_plugin): $(_test) $(_plugin)) \
+ $(eval RUN_TESTS+=run-plugin-$(_test)-with-$(_plugin)))
+
endif # MULTIARCH_TESTS
endif # CONFIG_PLUGIN
diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
index bfdf7197a7b..38345ff8805 100644
--- a/tests/tcg/multiarch/Makefile.target
+++ b/tests/tcg/multiarch/Makefile.target
@@ -189,6 +189,10 @@ run-plugin-semiconsole-with-%:
TESTS += semihosting semiconsole
endif
+test-plugin-mem-access: CFLAGS+=-pthread -O0
+test-plugin-mem-access: LDFLAGS+=-pthread -O0
+
+ifeq ($(CONFIG_PLUGIN),y)
# Test plugin memory access instrumentation
run-plugin-test-plugin-mem-access-with-libmem.so: \
PLUGIN_ARGS=$(COMMA)print-accesses=true
@@ -197,8 +201,8 @@ run-plugin-test-plugin-mem-access-with-libmem.so: \
$(SRC_PATH)/tests/tcg/multiarch/check-plugin-output.sh \
$(QEMU) $<
-test-plugin-mem-access: CFLAGS+=-pthread -O0
-test-plugin-mem-access: LDFLAGS+=-pthread -O0
+EXTRA_RUNS += run-plugin-test-plugin-mem-access-with-libmem.so
+endif
# Update TESTS
TESTS += $(MULTIARCH_TESTS)
diff --git a/tests/tcg/multiarch/system/Makefile.softmmu-target b/tests/tcg/multiarch/system/Makefile.softmmu-target
index 5acf2700812..4171b4e6aa0 100644
--- a/tests/tcg/multiarch/system/Makefile.softmmu-target
+++ b/tests/tcg/multiarch/system/Makefile.softmmu-target
@@ -71,8 +71,11 @@ endif
MULTIARCH_RUNS += run-gdbstub-memory run-gdbstub-interrupt \
run-gdbstub-untimely-packet run-gdbstub-registers
+ifeq ($(CONFIG_PLUGIN),y)
# Test plugin memory access instrumentation
-run-plugin-memory-with-libmem.so: \
- PLUGIN_ARGS=$(COMMA)region-summary=true
-run-plugin-memory-with-libmem.so: \
- CHECK_PLUGIN_OUTPUT_COMMAND=$(MULTIARCH_SYSTEM_SRC)/validate-memory-counts.py $@.out
+run-plugin-memory-with-libmem.so: memory libmem.so
+run-plugin-memory-with-libmem.so: PLUGIN_ARGS=$(COMMA)region-summary=true
+run-plugin-memory-with-libmem.so: CHECK_PLUGIN_OUTPUT_COMMAND=$(MULTIARCH_SYSTEM_SRC)/validate-memory-counts.py $@.out
+
+EXTRA_RUNS += run-plugin-memory-with-libmem.so
+endif
--
2.47.2
Hi Alex, On 7/27/25 1:32 AM, Alex Bennée wrote: > As our set of multiarch tests has grown the practice of running every > plugin with every test is becoming unsustainable. If we switch to > ensuring every test gets run with at least one plugin we can speed > things up. > > Some plugins do need to be run with specific tests (for example the > memory instrumentation test). We can handle this by manually adding > them to EXTRA_RUNS. We also need to wrap rules in a CONFIG_PLUGIN test > so we don't enable the runs when plugins are not enabled. > > Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Message-ID: <20250725154517.3523095-12-alex.bennee@linaro.org> > > diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target > index a12b15637ea..18afd5be194 100644 > --- a/tests/tcg/Makefile.target > +++ b/tests/tcg/Makefile.target > @@ -173,14 +173,25 @@ PLUGINS=$(filter-out $(DISABLE_PLUGINS), \ > # We need to ensure expand the run-plugin-TEST-with-PLUGIN > # pre-requistes manually here as we can't use stems to handle it. We > # only expand MULTIARCH_TESTS which are common on most of our targets > -# to avoid an exponential explosion as new tests are added. We also > -# add some special helpers the run-plugin- rules can use below. > +# and rotate the plugins so we don't grow too out of control as new > +# tests are added. Plugins that need to run with a specific test > +# should ensure they add their combination to EXTRA_RUNS. > > ifneq ($(MULTIARCH_TESTS),) > -$(foreach p,$(PLUGINS), \ > - $(foreach t,$(MULTIARCH_TESTS),\ > - $(eval run-plugin-$(t)-with-$(p): $t $p) \ > - $(eval RUN_TESTS+=run-plugin-$(t)-with-$(p)))) > + > +NUM_PLUGINS := $(words $(PLUGINS)) > +NUM_TESTS := $(words $(MULTIARCH_TESTS)) > + > +define mod_plus_one > + $(shell $(PYTHON) -c "print( ($(1) % $(2)) + 1 )") > +endef > + > +$(foreach _idx, $(shell seq 1 $(NUM_TESTS)), \ > + $(eval _test := $(word $(_idx), $(MULTIARCH_TESTS))) \ > + $(eval _plugin := $(word $(call mod_plus_one, $(_idx), $(NUM_PLUGINS)), $(PLUGINS))) \ > + $(eval run-plugin-$(_test)-with-$(_plugin): $(_test) $(_plugin)) \ > + $(eval RUN_TESTS+=run-plugin-$(_test)-with-$(_plugin))) > + > endif # MULTIARCH_TESTS > endif # CONFIG_PLUGIN > > diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target > index bfdf7197a7b..38345ff8805 100644 > --- a/tests/tcg/multiarch/Makefile.target > +++ b/tests/tcg/multiarch/Makefile.target > @@ -189,6 +189,10 @@ run-plugin-semiconsole-with-%: > TESTS += semihosting semiconsole > endif > > +test-plugin-mem-access: CFLAGS+=-pthread -O0 > +test-plugin-mem-access: LDFLAGS+=-pthread -O0 > + > +ifeq ($(CONFIG_PLUGIN),y) > # Test plugin memory access instrumentation > run-plugin-test-plugin-mem-access-with-libmem.so: \ > PLUGIN_ARGS=$(COMMA)print-accesses=true > @@ -197,8 +201,8 @@ run-plugin-test-plugin-mem-access-with-libmem.so: \ > $(SRC_PATH)/tests/tcg/multiarch/check-plugin-output.sh \ > $(QEMU) $< > > -test-plugin-mem-access: CFLAGS+=-pthread -O0 > -test-plugin-mem-access: LDFLAGS+=-pthread -O0 > +EXTRA_RUNS += run-plugin-test-plugin-mem-access-with-libmem.so > +endif > > # Update TESTS > TESTS += $(MULTIARCH_TESTS) > diff --git a/tests/tcg/multiarch/system/Makefile.softmmu-target b/tests/tcg/multiarch/system/Makefile.softmmu-target > index 5acf2700812..4171b4e6aa0 100644 > --- a/tests/tcg/multiarch/system/Makefile.softmmu-target > +++ b/tests/tcg/multiarch/system/Makefile.softmmu-target > @@ -71,8 +71,11 @@ endif > MULTIARCH_RUNS += run-gdbstub-memory run-gdbstub-interrupt \ > run-gdbstub-untimely-packet run-gdbstub-registers > > +ifeq ($(CONFIG_PLUGIN),y) > # Test plugin memory access instrumentation > -run-plugin-memory-with-libmem.so: \ > - PLUGIN_ARGS=$(COMMA)region-summary=true > -run-plugin-memory-with-libmem.so: \ > - CHECK_PLUGIN_OUTPUT_COMMAND=$(MULTIARCH_SYSTEM_SRC)/validate-memory-counts.py $@.out > +run-plugin-memory-with-libmem.so: memory libmem.so > +run-plugin-memory-with-libmem.so: PLUGIN_ARGS=$(COMMA)region-summary=true > +run-plugin-memory-with-libmem.so: CHECK_PLUGIN_OUTPUT_COMMAND=$(MULTIARCH_SYSTEM_SRC)/validate-memory-counts.py $@.out > + > +EXTRA_RUNS += run-plugin-memory-with-libmem.so > +endif I'm not sure how it's related, but check-tcg on aarch64 host now fails [1] since this series was merged, and I suspect it may be related to this patch. I didn't spend time to reproduce and investigate it. [1] https://github.com/pbo-linaro/qemu-ci/actions/runs/16575679153/job/46879690693
On 7/28/25 12:03 PM, Pierrick Bouvier wrote: > Hi Alex, > > On 7/27/25 1:32 AM, Alex Bennée wrote: >> As our set of multiarch tests has grown the practice of running every >> plugin with every test is becoming unsustainable. If we switch to >> ensuring every test gets run with at least one plugin we can speed >> things up. >> >> Some plugins do need to be run with specific tests (for example the >> memory instrumentation test). We can handle this by manually adding >> them to EXTRA_RUNS. We also need to wrap rules in a CONFIG_PLUGIN test >> so we don't enable the runs when plugins are not enabled. >> >> Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> Message-ID: <20250725154517.3523095-12-alex.bennee@linaro.org> >> >> diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target >> index a12b15637ea..18afd5be194 100644 >> --- a/tests/tcg/Makefile.target >> +++ b/tests/tcg/Makefile.target >> @@ -173,14 +173,25 @@ PLUGINS=$(filter-out $(DISABLE_PLUGINS), \ >> # We need to ensure expand the run-plugin-TEST-with-PLUGIN >> # pre-requistes manually here as we can't use stems to handle it. We >> # only expand MULTIARCH_TESTS which are common on most of our targets >> -# to avoid an exponential explosion as new tests are added. We also >> -# add some special helpers the run-plugin- rules can use below. >> +# and rotate the plugins so we don't grow too out of control as new >> +# tests are added. Plugins that need to run with a specific test >> +# should ensure they add their combination to EXTRA_RUNS. >> >> ifneq ($(MULTIARCH_TESTS),) >> -$(foreach p,$(PLUGINS), \ >> - $(foreach t,$(MULTIARCH_TESTS),\ >> - $(eval run-plugin-$(t)-with-$(p): $t $p) \ >> - $(eval RUN_TESTS+=run-plugin-$(t)-with-$(p)))) >> + >> +NUM_PLUGINS := $(words $(PLUGINS)) >> +NUM_TESTS := $(words $(MULTIARCH_TESTS)) >> + >> +define mod_plus_one >> + $(shell $(PYTHON) -c "print( ($(1) % $(2)) + 1 )") >> +endef >> + >> +$(foreach _idx, $(shell seq 1 $(NUM_TESTS)), \ >> + $(eval _test := $(word $(_idx), $(MULTIARCH_TESTS))) \ >> + $(eval _plugin := $(word $(call mod_plus_one, $(_idx), $(NUM_PLUGINS)), $(PLUGINS))) \ >> + $(eval run-plugin-$(_test)-with-$(_plugin): $(_test) $(_plugin)) \ >> + $(eval RUN_TESTS+=run-plugin-$(_test)-with-$(_plugin))) >> + >> endif # MULTIARCH_TESTS >> endif # CONFIG_PLUGIN >> >> diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target >> index bfdf7197a7b..38345ff8805 100644 >> --- a/tests/tcg/multiarch/Makefile.target >> +++ b/tests/tcg/multiarch/Makefile.target >> @@ -189,6 +189,10 @@ run-plugin-semiconsole-with-%: >> TESTS += semihosting semiconsole >> endif >> >> +test-plugin-mem-access: CFLAGS+=-pthread -O0 >> +test-plugin-mem-access: LDFLAGS+=-pthread -O0 >> + >> +ifeq ($(CONFIG_PLUGIN),y) >> # Test plugin memory access instrumentation >> run-plugin-test-plugin-mem-access-with-libmem.so: \ >> PLUGIN_ARGS=$(COMMA)print-accesses=true >> @@ -197,8 +201,8 @@ run-plugin-test-plugin-mem-access-with-libmem.so: \ >> $(SRC_PATH)/tests/tcg/multiarch/check-plugin-output.sh \ >> $(QEMU) $< >> >> -test-plugin-mem-access: CFLAGS+=-pthread -O0 >> -test-plugin-mem-access: LDFLAGS+=-pthread -O0 >> +EXTRA_RUNS += run-plugin-test-plugin-mem-access-with-libmem.so >> +endif >> >> # Update TESTS >> TESTS += $(MULTIARCH_TESTS) >> diff --git a/tests/tcg/multiarch/system/Makefile.softmmu-target b/tests/tcg/multiarch/system/Makefile.softmmu-target >> index 5acf2700812..4171b4e6aa0 100644 >> --- a/tests/tcg/multiarch/system/Makefile.softmmu-target >> +++ b/tests/tcg/multiarch/system/Makefile.softmmu-target >> @@ -71,8 +71,11 @@ endif >> MULTIARCH_RUNS += run-gdbstub-memory run-gdbstub-interrupt \ >> run-gdbstub-untimely-packet run-gdbstub-registers >> >> +ifeq ($(CONFIG_PLUGIN),y) >> # Test plugin memory access instrumentation >> -run-plugin-memory-with-libmem.so: \ >> - PLUGIN_ARGS=$(COMMA)region-summary=true >> -run-plugin-memory-with-libmem.so: \ >> - CHECK_PLUGIN_OUTPUT_COMMAND=$(MULTIARCH_SYSTEM_SRC)/validate-memory-counts.py $@.out >> +run-plugin-memory-with-libmem.so: memory libmem.so >> +run-plugin-memory-with-libmem.so: PLUGIN_ARGS=$(COMMA)region-summary=true >> +run-plugin-memory-with-libmem.so: CHECK_PLUGIN_OUTPUT_COMMAND=$(MULTIARCH_SYSTEM_SRC)/validate-memory-counts.py $@.out >> + >> +EXTRA_RUNS += run-plugin-memory-with-libmem.so >> +endif > > I'm not sure how it's related, but check-tcg on aarch64 host now fails > [1] since this series was merged, and I suspect it may be related to > this patch. I didn't spend time to reproduce and investigate it. > Reverting this patch solved the problem on my side. As well, I'm not sure if it's a real problem to run all plugins for all tests. At the moment, it takes 5 min on a slow machine with 4 cpus and a sanitized build, something we can probably live with for now. Unfortunately Alex is out this week, but it would be nice if another maintainer could make a PR reverting this patch. Regards, Pierrick > [1] > https://github.com/pbo-linaro/qemu-ci/actions/runs/16575679153/job/46879690693
Pierrick Bouvier <pierrick.bouvier@linaro.org> writes: > On 7/28/25 12:03 PM, Pierrick Bouvier wrote: >> Hi Alex, >> On 7/27/25 1:32 AM, Alex Bennée wrote: >>> As our set of multiarch tests has grown the practice of running every >>> plugin with every test is becoming unsustainable. If we switch to >>> ensuring every test gets run with at least one plugin we can speed >>> things up. >>> >>> Some plugins do need to be run with specific tests (for example the >>> memory instrumentation test). We can handle this by manually adding >>> them to EXTRA_RUNS. We also need to wrap rules in a CONFIG_PLUGIN test >>> so we don't enable the runs when plugins are not enabled. >>> >>> Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org> >>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >>> Message-ID: <20250725154517.3523095-12-alex.bennee@linaro.org> >>> >>> diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target >>> index a12b15637ea..18afd5be194 100644 >>> --- a/tests/tcg/Makefile.target >>> +++ b/tests/tcg/Makefile.target >>> @@ -173,14 +173,25 @@ PLUGINS=$(filter-out $(DISABLE_PLUGINS), \ >>> # We need to ensure expand the run-plugin-TEST-with-PLUGIN >>> # pre-requistes manually here as we can't use stems to handle it. We >>> # only expand MULTIARCH_TESTS which are common on most of our targets >>> -# to avoid an exponential explosion as new tests are added. We also >>> -# add some special helpers the run-plugin- rules can use below. >>> +# and rotate the plugins so we don't grow too out of control as new >>> +# tests are added. Plugins that need to run with a specific test >>> +# should ensure they add their combination to EXTRA_RUNS. >>> ifneq ($(MULTIARCH_TESTS),) >>> -$(foreach p,$(PLUGINS), \ >>> - $(foreach t,$(MULTIARCH_TESTS),\ >>> - $(eval run-plugin-$(t)-with-$(p): $t $p) \ >>> - $(eval RUN_TESTS+=run-plugin-$(t)-with-$(p)))) >>> + >>> +NUM_PLUGINS := $(words $(PLUGINS)) >>> +NUM_TESTS := $(words $(MULTIARCH_TESTS)) >>> + >>> +define mod_plus_one >>> + $(shell $(PYTHON) -c "print( ($(1) % $(2)) + 1 )") >>> +endef >>> + >>> +$(foreach _idx, $(shell seq 1 $(NUM_TESTS)), \ >>> + $(eval _test := $(word $(_idx), $(MULTIARCH_TESTS))) \ >>> + $(eval _plugin := $(word $(call mod_plus_one, $(_idx), $(NUM_PLUGINS)), $(PLUGINS))) \ >>> + $(eval run-plugin-$(_test)-with-$(_plugin): $(_test) $(_plugin)) \ >>> + $(eval RUN_TESTS+=run-plugin-$(_test)-with-$(_plugin))) >>> + >>> endif # MULTIARCH_TESTS >>> endif # CONFIG_PLUGIN >>> diff --git a/tests/tcg/multiarch/Makefile.target >>> b/tests/tcg/multiarch/Makefile.target >>> index bfdf7197a7b..38345ff8805 100644 >>> --- a/tests/tcg/multiarch/Makefile.target >>> +++ b/tests/tcg/multiarch/Makefile.target >>> @@ -189,6 +189,10 @@ run-plugin-semiconsole-with-%: >>> TESTS += semihosting semiconsole >>> endif >>> +test-plugin-mem-access: CFLAGS+=-pthread -O0 >>> +test-plugin-mem-access: LDFLAGS+=-pthread -O0 >>> + >>> +ifeq ($(CONFIG_PLUGIN),y) >>> # Test plugin memory access instrumentation >>> run-plugin-test-plugin-mem-access-with-libmem.so: \ >>> PLUGIN_ARGS=$(COMMA)print-accesses=true >>> @@ -197,8 +201,8 @@ run-plugin-test-plugin-mem-access-with-libmem.so: \ >>> $(SRC_PATH)/tests/tcg/multiarch/check-plugin-output.sh \ >>> $(QEMU) $< >>> -test-plugin-mem-access: CFLAGS+=-pthread -O0 >>> -test-plugin-mem-access: LDFLAGS+=-pthread -O0 >>> +EXTRA_RUNS += run-plugin-test-plugin-mem-access-with-libmem.so >>> +endif >>> # Update TESTS >>> TESTS += $(MULTIARCH_TESTS) >>> diff --git a/tests/tcg/multiarch/system/Makefile.softmmu-target b/tests/tcg/multiarch/system/Makefile.softmmu-target >>> index 5acf2700812..4171b4e6aa0 100644 >>> --- a/tests/tcg/multiarch/system/Makefile.softmmu-target >>> +++ b/tests/tcg/multiarch/system/Makefile.softmmu-target >>> @@ -71,8 +71,11 @@ endif >>> MULTIARCH_RUNS += run-gdbstub-memory run-gdbstub-interrupt \ >>> run-gdbstub-untimely-packet run-gdbstub-registers >>> +ifeq ($(CONFIG_PLUGIN),y) >>> # Test plugin memory access instrumentation >>> -run-plugin-memory-with-libmem.so: \ >>> - PLUGIN_ARGS=$(COMMA)region-summary=true >>> -run-plugin-memory-with-libmem.so: \ >>> - CHECK_PLUGIN_OUTPUT_COMMAND=$(MULTIARCH_SYSTEM_SRC)/validate-memory-counts.py $@.out >>> +run-plugin-memory-with-libmem.so: memory libmem.so >>> +run-plugin-memory-with-libmem.so: PLUGIN_ARGS=$(COMMA)region-summary=true >>> +run-plugin-memory-with-libmem.so: CHECK_PLUGIN_OUTPUT_COMMAND=$(MULTIARCH_SYSTEM_SRC)/validate-memory-counts.py $@.out >>> + >>> +EXTRA_RUNS += run-plugin-memory-with-libmem.so >>> +endif >> I'm not sure how it's related, but check-tcg on aarch64 host now >> fails >> [1] since this series was merged, and I suspect it may be related to >> this patch. I didn't spend time to reproduce and investigate it. >> > > Reverting this patch solved the problem on my side. As well, I'm not > sure if it's a real problem to run all plugins for all tests. At the > moment, it takes 5 min on a slow machine with 4 cpus and a sanitized > build, something we can probably live with for now. It definitely is - but it really shows up on linux-user builds because we have an increasing number of multiarch tests and its not sustainable to increase by $NARCH * $NPLUGIN everytime a new multiarch test is added. > > Unfortunately Alex is out this week, but it would be nice if another > maintainer could make a PR reverting this patch. > > Regards, > Pierrick > >> [1] >> https://github.com/pbo-linaro/qemu-ci/actions/runs/16575679153/job/46879690693 -- Alex Bennée Virtualisation Tech Lead @ Linaro
On 8/4/25 1:39 AM, Alex Bennée wrote: > Pierrick Bouvier <pierrick.bouvier@linaro.org> writes: > >> On 7/28/25 12:03 PM, Pierrick Bouvier wrote: >>> Hi Alex, >>> On 7/27/25 1:32 AM, Alex Bennée wrote: >>>> As our set of multiarch tests has grown the practice of running every >>>> plugin with every test is becoming unsustainable. If we switch to >>>> ensuring every test gets run with at least one plugin we can speed >>>> things up. >>>> >>>> Some plugins do need to be run with specific tests (for example the >>>> memory instrumentation test). We can handle this by manually adding >>>> them to EXTRA_RUNS. We also need to wrap rules in a CONFIG_PLUGIN test >>>> so we don't enable the runs when plugins are not enabled. >>>> >>>> Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org> >>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >>>> Message-ID: <20250725154517.3523095-12-alex.bennee@linaro.org> >>>> >>>> diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target >>>> index a12b15637ea..18afd5be194 100644 >>>> --- a/tests/tcg/Makefile.target >>>> +++ b/tests/tcg/Makefile.target >>>> @@ -173,14 +173,25 @@ PLUGINS=$(filter-out $(DISABLE_PLUGINS), \ >>>> # We need to ensure expand the run-plugin-TEST-with-PLUGIN >>>> # pre-requistes manually here as we can't use stems to handle it. We >>>> # only expand MULTIARCH_TESTS which are common on most of our targets >>>> -# to avoid an exponential explosion as new tests are added. We also >>>> -# add some special helpers the run-plugin- rules can use below. >>>> +# and rotate the plugins so we don't grow too out of control as new >>>> +# tests are added. Plugins that need to run with a specific test >>>> +# should ensure they add their combination to EXTRA_RUNS. >>>> ifneq ($(MULTIARCH_TESTS),) >>>> -$(foreach p,$(PLUGINS), \ >>>> - $(foreach t,$(MULTIARCH_TESTS),\ >>>> - $(eval run-plugin-$(t)-with-$(p): $t $p) \ >>>> - $(eval RUN_TESTS+=run-plugin-$(t)-with-$(p)))) >>>> + >>>> +NUM_PLUGINS := $(words $(PLUGINS)) >>>> +NUM_TESTS := $(words $(MULTIARCH_TESTS)) >>>> + >>>> +define mod_plus_one >>>> + $(shell $(PYTHON) -c "print( ($(1) % $(2)) + 1 )") >>>> +endef >>>> + >>>> +$(foreach _idx, $(shell seq 1 $(NUM_TESTS)), \ >>>> + $(eval _test := $(word $(_idx), $(MULTIARCH_TESTS))) \ >>>> + $(eval _plugin := $(word $(call mod_plus_one, $(_idx), $(NUM_PLUGINS)), $(PLUGINS))) \ >>>> + $(eval run-plugin-$(_test)-with-$(_plugin): $(_test) $(_plugin)) \ >>>> + $(eval RUN_TESTS+=run-plugin-$(_test)-with-$(_plugin))) >>>> + >>>> endif # MULTIARCH_TESTS >>>> endif # CONFIG_PLUGIN >>>> diff --git a/tests/tcg/multiarch/Makefile.target >>>> b/tests/tcg/multiarch/Makefile.target >>>> index bfdf7197a7b..38345ff8805 100644 >>>> --- a/tests/tcg/multiarch/Makefile.target >>>> +++ b/tests/tcg/multiarch/Makefile.target >>>> @@ -189,6 +189,10 @@ run-plugin-semiconsole-with-%: >>>> TESTS += semihosting semiconsole >>>> endif >>>> +test-plugin-mem-access: CFLAGS+=-pthread -O0 >>>> +test-plugin-mem-access: LDFLAGS+=-pthread -O0 >>>> + >>>> +ifeq ($(CONFIG_PLUGIN),y) >>>> # Test plugin memory access instrumentation >>>> run-plugin-test-plugin-mem-access-with-libmem.so: \ >>>> PLUGIN_ARGS=$(COMMA)print-accesses=true >>>> @@ -197,8 +201,8 @@ run-plugin-test-plugin-mem-access-with-libmem.so: \ >>>> $(SRC_PATH)/tests/tcg/multiarch/check-plugin-output.sh \ >>>> $(QEMU) $< >>>> -test-plugin-mem-access: CFLAGS+=-pthread -O0 >>>> -test-plugin-mem-access: LDFLAGS+=-pthread -O0 >>>> +EXTRA_RUNS += run-plugin-test-plugin-mem-access-with-libmem.so >>>> +endif >>>> # Update TESTS >>>> TESTS += $(MULTIARCH_TESTS) >>>> diff --git a/tests/tcg/multiarch/system/Makefile.softmmu-target b/tests/tcg/multiarch/system/Makefile.softmmu-target >>>> index 5acf2700812..4171b4e6aa0 100644 >>>> --- a/tests/tcg/multiarch/system/Makefile.softmmu-target >>>> +++ b/tests/tcg/multiarch/system/Makefile.softmmu-target >>>> @@ -71,8 +71,11 @@ endif >>>> MULTIARCH_RUNS += run-gdbstub-memory run-gdbstub-interrupt \ >>>> run-gdbstub-untimely-packet run-gdbstub-registers >>>> +ifeq ($(CONFIG_PLUGIN),y) >>>> # Test plugin memory access instrumentation >>>> -run-plugin-memory-with-libmem.so: \ >>>> - PLUGIN_ARGS=$(COMMA)region-summary=true >>>> -run-plugin-memory-with-libmem.so: \ >>>> - CHECK_PLUGIN_OUTPUT_COMMAND=$(MULTIARCH_SYSTEM_SRC)/validate-memory-counts.py $@.out >>>> +run-plugin-memory-with-libmem.so: memory libmem.so >>>> +run-plugin-memory-with-libmem.so: PLUGIN_ARGS=$(COMMA)region-summary=true >>>> +run-plugin-memory-with-libmem.so: CHECK_PLUGIN_OUTPUT_COMMAND=$(MULTIARCH_SYSTEM_SRC)/validate-memory-counts.py $@.out >>>> + >>>> +EXTRA_RUNS += run-plugin-memory-with-libmem.so >>>> +endif >>> I'm not sure how it's related, but check-tcg on aarch64 host now >>> fails >>> [1] since this series was merged, and I suspect it may be related to >>> this patch. I didn't spend time to reproduce and investigate it. >>> >> >> Reverting this patch solved the problem on my side. As well, I'm not >> sure if it's a real problem to run all plugins for all tests. At the >> moment, it takes 5 min on a slow machine with 4 cpus and a sanitized >> build, something we can probably live with for now. > > It definitely is - but it really shows up on linux-user builds because > we have an increasing number of multiarch tests and its not sustainable > to increase by $NARCH * $NPLUGIN everytime a new multiarch test is > added. > For information, plugins tests are responsible for 65% of check-tcg time approximately, so this makes sense. >> >> Unfortunately Alex is out this week, but it would be nice if another >> maintainer could make a PR reverting this patch. >> >> Regards, >> Pierrick >> >>> [1] >>> https://github.com/pbo-linaro/qemu-ci/actions/runs/16575679153/job/46879690693 >
Hi, On 7/28/25 16:03, Pierrick Bouvier wrote: > Hi Alex, > > On 7/27/25 1:32 AM, Alex Bennée wrote: >> As our set of multiarch tests has grown the practice of running every >> plugin with every test is becoming unsustainable. If we switch to >> ensuring every test gets run with at least one plugin we can speed >> things up. >> >> Some plugins do need to be run with specific tests (for example the >> memory instrumentation test). We can handle this by manually adding >> them to EXTRA_RUNS. We also need to wrap rules in a CONFIG_PLUGIN test >> so we don't enable the runs when plugins are not enabled. >> >> Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> Message-ID: <20250725154517.3523095-12-alex.bennee@linaro.org> >> >> diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target >> index a12b15637ea..18afd5be194 100644 >> --- a/tests/tcg/Makefile.target >> +++ b/tests/tcg/Makefile.target >> @@ -173,14 +173,25 @@ PLUGINS=$(filter-out $(DISABLE_PLUGINS), \ >> # We need to ensure expand the run-plugin-TEST-with-PLUGIN >> # pre-requistes manually here as we can't use stems to handle it. We >> # only expand MULTIARCH_TESTS which are common on most of our targets >> -# to avoid an exponential explosion as new tests are added. We also >> -# add some special helpers the run-plugin- rules can use below. >> +# and rotate the plugins so we don't grow too out of control as new >> +# tests are added. Plugins that need to run with a specific test >> +# should ensure they add their combination to EXTRA_RUNS. >> ifneq ($(MULTIARCH_TESTS),) >> -$(foreach p,$(PLUGINS), \ >> - $(foreach t,$(MULTIARCH_TESTS),\ >> - $(eval run-plugin-$(t)-with-$(p): $t $p) \ >> - $(eval RUN_TESTS+=run-plugin-$(t)-with-$(p)))) >> + >> +NUM_PLUGINS := $(words $(PLUGINS)) >> +NUM_TESTS := $(words $(MULTIARCH_TESTS)) >> + >> +define mod_plus_one >> + $(shell $(PYTHON) -c "print( ($(1) % $(2)) + 1 )") >> +endef >> + >> +$(foreach _idx, $(shell seq 1 $(NUM_TESTS)), \ >> + $(eval _test := $(word $(_idx), $(MULTIARCH_TESTS))) \ >> + $(eval _plugin := $(word $(call mod_plus_one, $(_idx), $(NUM_PLUGINS)), $(PLUGINS))) \ >> + $(eval run-plugin-$(_test)-with-$(_plugin): $(_test) $(_plugin)) \ >> + $(eval RUN_TESTS+=run-plugin-$(_test)-with-$(_plugin))) >> + >> endif # MULTIARCH_TESTS >> endif # CONFIG_PLUGIN >> diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target >> index bfdf7197a7b..38345ff8805 100644 >> --- a/tests/tcg/multiarch/Makefile.target >> +++ b/tests/tcg/multiarch/Makefile.target >> @@ -189,6 +189,10 @@ run-plugin-semiconsole-with-%: >> TESTS += semihosting semiconsole >> endif >> +test-plugin-mem-access: CFLAGS+=-pthread -O0 >> +test-plugin-mem-access: LDFLAGS+=-pthread -O0 >> + >> +ifeq ($(CONFIG_PLUGIN),y) >> # Test plugin memory access instrumentation >> run-plugin-test-plugin-mem-access-with-libmem.so: \ >> PLUGIN_ARGS=$(COMMA)print-accesses=true >> @@ -197,8 +201,8 @@ run-plugin-test-plugin-mem-access-with-libmem.so: \ >> $(SRC_PATH)/tests/tcg/multiarch/check-plugin-output.sh \ >> $(QEMU) $< >> -test-plugin-mem-access: CFLAGS+=-pthread -O0 >> -test-plugin-mem-access: LDFLAGS+=-pthread -O0 >> +EXTRA_RUNS += run-plugin-test-plugin-mem-access-with-libmem.so >> +endif >> # Update TESTS >> TESTS += $(MULTIARCH_TESTS) >> diff --git a/tests/tcg/multiarch/system/Makefile.softmmu-target b/tests/tcg/multiarch/system/Makefile.softmmu-target >> index 5acf2700812..4171b4e6aa0 100644 >> --- a/tests/tcg/multiarch/system/Makefile.softmmu-target >> +++ b/tests/tcg/multiarch/system/Makefile.softmmu-target >> @@ -71,8 +71,11 @@ endif >> MULTIARCH_RUNS += run-gdbstub-memory run-gdbstub-interrupt \ >> run-gdbstub-untimely-packet run-gdbstub-registers >> +ifeq ($(CONFIG_PLUGIN),y) >> # Test plugin memory access instrumentation >> -run-plugin-memory-with-libmem.so: \ >> - PLUGIN_ARGS=$(COMMA)region-summary=true >> -run-plugin-memory-with-libmem.so: \ >> - CHECK_PLUGIN_OUTPUT_COMMAND=$(MULTIARCH_SYSTEM_SRC)/validate-memory-counts.py $@.out >> +run-plugin-memory-with-libmem.so: memory libmem.so >> +run-plugin-memory-with-libmem.so: PLUGIN_ARGS=$(COMMA)region-summary=true >> +run-plugin-memory-with-libmem.so: CHECK_PLUGIN_OUTPUT_COMMAND=$(MULTIARCH_SYSTEM_SRC)/validate-memory-counts.py $@.out >> + >> +EXTRA_RUNS += run-plugin-memory-with-libmem.so >> +endif > > I'm not sure how it's related, but check-tcg on aarch64 host now fails [1] since this series was merged, and I suspect it may be related to this patch. I didn't spend time to reproduce and investigate it. > > [1] https://github.com/pbo-linaro/qemu-ci/actions/runs/16575679153/job/46879690693 I was not able to reproduce it locally on 22.04, but in the CI indeed the test command is missing the "test-plugin-mem-access" binary at the end, just before redirection to .so.out, it should be: [...] -D test-plugin-mem-access-with-libmem.so.pout test-plugin-mem-access > run-plugin-test-plugin-mem-access-with-libmem.so.out not: [...] -D test-plugin-mem-access-with-libmem.so.pout > run-plugin-test-plugin-mem-access-with-libmem.so.out Cheers, Gustavo
Hi, On 7/28/25 17:14, Gustavo Romero wrote: > Hi, > > On 7/28/25 16:03, Pierrick Bouvier wrote: >> Hi Alex, >> >> On 7/27/25 1:32 AM, Alex Bennée wrote: >>> As our set of multiarch tests has grown the practice of running every >>> plugin with every test is becoming unsustainable. If we switch to >>> ensuring every test gets run with at least one plugin we can speed >>> things up. >>> >>> Some plugins do need to be run with specific tests (for example the >>> memory instrumentation test). We can handle this by manually adding >>> them to EXTRA_RUNS. We also need to wrap rules in a CONFIG_PLUGIN test >>> so we don't enable the runs when plugins are not enabled. >>> >>> Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org> >>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >>> Message-ID: <20250725154517.3523095-12-alex.bennee@linaro.org> >>> >>> diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target >>> index a12b15637ea..18afd5be194 100644 >>> --- a/tests/tcg/Makefile.target >>> +++ b/tests/tcg/Makefile.target >>> @@ -173,14 +173,25 @@ PLUGINS=$(filter-out $(DISABLE_PLUGINS), \ >>> # We need to ensure expand the run-plugin-TEST-with-PLUGIN >>> # pre-requistes manually here as we can't use stems to handle it. We >>> # only expand MULTIARCH_TESTS which are common on most of our targets >>> -# to avoid an exponential explosion as new tests are added. We also >>> -# add some special helpers the run-plugin- rules can use below. >>> +# and rotate the plugins so we don't grow too out of control as new >>> +# tests are added. Plugins that need to run with a specific test >>> +# should ensure they add their combination to EXTRA_RUNS. >>> ifneq ($(MULTIARCH_TESTS),) >>> -$(foreach p,$(PLUGINS), \ >>> - $(foreach t,$(MULTIARCH_TESTS),\ >>> - $(eval run-plugin-$(t)-with-$(p): $t $p) \ >>> - $(eval RUN_TESTS+=run-plugin-$(t)-with-$(p)))) >>> + >>> +NUM_PLUGINS := $(words $(PLUGINS)) >>> +NUM_TESTS := $(words $(MULTIARCH_TESTS)) >>> + >>> +define mod_plus_one >>> + $(shell $(PYTHON) -c "print( ($(1) % $(2)) + 1 )") >>> +endef >>> + >>> +$(foreach _idx, $(shell seq 1 $(NUM_TESTS)), \ >>> + $(eval _test := $(word $(_idx), $(MULTIARCH_TESTS))) \ >>> + $(eval _plugin := $(word $(call mod_plus_one, $(_idx), $(NUM_PLUGINS)), $(PLUGINS))) \ >>> + $(eval run-plugin-$(_test)-with-$(_plugin): $(_test) $(_plugin)) \ >>> + $(eval RUN_TESTS+=run-plugin-$(_test)-with-$(_plugin))) >>> + >>> endif # MULTIARCH_TESTS >>> endif # CONFIG_PLUGIN >>> diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target >>> index bfdf7197a7b..38345ff8805 100644 >>> --- a/tests/tcg/multiarch/Makefile.target >>> +++ b/tests/tcg/multiarch/Makefile.target >>> @@ -189,6 +189,10 @@ run-plugin-semiconsole-with-%: >>> TESTS += semihosting semiconsole >>> endif >>> +test-plugin-mem-access: CFLAGS+=-pthread -O0 >>> +test-plugin-mem-access: LDFLAGS+=-pthread -O0 >>> + >>> +ifeq ($(CONFIG_PLUGIN),y) >>> # Test plugin memory access instrumentation >>> run-plugin-test-plugin-mem-access-with-libmem.so: \ >>> PLUGIN_ARGS=$(COMMA)print-accesses=true >>> @@ -197,8 +201,8 @@ run-plugin-test-plugin-mem-access-with-libmem.so: \ >>> $(SRC_PATH)/tests/tcg/multiarch/check-plugin-output.sh \ >>> $(QEMU) $< >>> -test-plugin-mem-access: CFLAGS+=-pthread -O0 >>> -test-plugin-mem-access: LDFLAGS+=-pthread -O0 >>> +EXTRA_RUNS += run-plugin-test-plugin-mem-access-with-libmem.so >>> +endif >>> # Update TESTS >>> TESTS += $(MULTIARCH_TESTS) >>> diff --git a/tests/tcg/multiarch/system/Makefile.softmmu-target b/tests/tcg/multiarch/system/Makefile.softmmu-target >>> index 5acf2700812..4171b4e6aa0 100644 >>> --- a/tests/tcg/multiarch/system/Makefile.softmmu-target >>> +++ b/tests/tcg/multiarch/system/Makefile.softmmu-target >>> @@ -71,8 +71,11 @@ endif >>> MULTIARCH_RUNS += run-gdbstub-memory run-gdbstub-interrupt \ >>> run-gdbstub-untimely-packet run-gdbstub-registers >>> +ifeq ($(CONFIG_PLUGIN),y) >>> # Test plugin memory access instrumentation >>> -run-plugin-memory-with-libmem.so: \ >>> - PLUGIN_ARGS=$(COMMA)region-summary=true >>> -run-plugin-memory-with-libmem.so: \ >>> - CHECK_PLUGIN_OUTPUT_COMMAND=$(MULTIARCH_SYSTEM_SRC)/validate-memory-counts.py $@.out >>> +run-plugin-memory-with-libmem.so: memory libmem.so >>> +run-plugin-memory-with-libmem.so: PLUGIN_ARGS=$(COMMA)region-summary=true >>> +run-plugin-memory-with-libmem.so: CHECK_PLUGIN_OUTPUT_COMMAND=$(MULTIARCH_SYSTEM_SRC)/validate-memory-counts.py $@.out >>> + >>> +EXTRA_RUNS += run-plugin-memory-with-libmem.so >>> +endif >> >> I'm not sure how it's related, but check-tcg on aarch64 host now fails [1] since this series was merged, and I suspect it may be related to this patch. I didn't spend time to reproduce and investigate it. >> >> [1] https://github.com/pbo-linaro/qemu-ci/actions/runs/16575679153/job/46879690693 > > I was not able to reproduce it locally on 22.04, but in the CI indeed the test command is missing the "test-plugin-mem-access" binary at the end, just before redirection to .so.out, it should be: > > [...] -D test-plugin-mem-access-with-libmem.so.pout test-plugin-mem-access > run-plugin-test-plugin-mem-access-with-libmem.so.out > > not: > > [...] -D test-plugin-mem-access-with-libmem.so.pout > run-plugin-test-plugin-mem-access-with-libmem.so.out The problem is that the new rules generated by a shuffled combination of tests and plugin (the one using eval) sometimes does not include the rule for the test+plugin combination passed via EXTRA_RUNs. So EXTRA_RUNS tests like run-plugin-test-plugin-mem-access-with-libmem.so might end up with a proper rule if the test test-plugin-mem-access is combined with other plugin randomly, since now a plugin is picked up based on a tests index _idx modulo with the number of plugins. A possible fix is to generate correctly the rules for the tests with plugins passed via EXTRA_RUNS and ideally, following the patch's mood, remove the test from running if any other plugin (no shuffle with any other plugin). Of course, as Pierrick said, this patch could be a premature optimization. So maybe it doesn´t justify adding more complexity to our Makefile (Makefiles are hard to debug, with 'eval' tricks, it's _reallly_ hard, so the more we avoid it the better). That said, if we want to keep this commit, I've kicked off a test to fix it here: https://gitlab.com/gusbromero/qemu/-/pipelines/1959953122 and sent the fix to the ML too. HTH. Cheers, Gustavo
On 7/31/25 11:58 AM, Gustavo Romero wrote: > Hi, > > On 7/28/25 17:14, Gustavo Romero wrote: >> Hi, >> >> On 7/28/25 16:03, Pierrick Bouvier wrote: >>> Hi Alex, >>> >>> On 7/27/25 1:32 AM, Alex Bennée wrote: >>>> As our set of multiarch tests has grown the practice of running every >>>> plugin with every test is becoming unsustainable. If we switch to >>>> ensuring every test gets run with at least one plugin we can speed >>>> things up. >>>> >>>> Some plugins do need to be run with specific tests (for example the >>>> memory instrumentation test). We can handle this by manually adding >>>> them to EXTRA_RUNS. We also need to wrap rules in a CONFIG_PLUGIN test >>>> so we don't enable the runs when plugins are not enabled. >>>> >>>> Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org> >>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >>>> Message-ID: <20250725154517.3523095-12-alex.bennee@linaro.org> >>>> >>>> diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target >>>> index a12b15637ea..18afd5be194 100644 >>>> --- a/tests/tcg/Makefile.target >>>> +++ b/tests/tcg/Makefile.target >>>> @@ -173,14 +173,25 @@ PLUGINS=$(filter-out $(DISABLE_PLUGINS), \ >>>> # We need to ensure expand the run-plugin-TEST-with-PLUGIN >>>> # pre-requistes manually here as we can't use stems to handle it. We >>>> # only expand MULTIARCH_TESTS which are common on most of our targets >>>> -# to avoid an exponential explosion as new tests are added. We also >>>> -# add some special helpers the run-plugin- rules can use below. >>>> +# and rotate the plugins so we don't grow too out of control as new >>>> +# tests are added. Plugins that need to run with a specific test >>>> +# should ensure they add their combination to EXTRA_RUNS. >>>> ifneq ($(MULTIARCH_TESTS),) >>>> -$(foreach p,$(PLUGINS), \ >>>> - $(foreach t,$(MULTIARCH_TESTS),\ >>>> - $(eval run-plugin-$(t)-with-$(p): $t $p) \ >>>> - $(eval RUN_TESTS+=run-plugin-$(t)-with-$(p)))) >>>> + >>>> +NUM_PLUGINS := $(words $(PLUGINS)) >>>> +NUM_TESTS := $(words $(MULTIARCH_TESTS)) >>>> + >>>> +define mod_plus_one >>>> + $(shell $(PYTHON) -c "print( ($(1) % $(2)) + 1 )") >>>> +endef >>>> + >>>> +$(foreach _idx, $(shell seq 1 $(NUM_TESTS)), \ >>>> + $(eval _test := $(word $(_idx), $(MULTIARCH_TESTS))) \ >>>> + $(eval _plugin := $(word $(call mod_plus_one, $(_idx), $(NUM_PLUGINS)), $(PLUGINS))) \ >>>> + $(eval run-plugin-$(_test)-with-$(_plugin): $(_test) $(_plugin)) \ >>>> + $(eval RUN_TESTS+=run-plugin-$(_test)-with-$(_plugin))) >>>> + >>>> endif # MULTIARCH_TESTS >>>> endif # CONFIG_PLUGIN >>>> diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target >>>> index bfdf7197a7b..38345ff8805 100644 >>>> --- a/tests/tcg/multiarch/Makefile.target >>>> +++ b/tests/tcg/multiarch/Makefile.target >>>> @@ -189,6 +189,10 @@ run-plugin-semiconsole-with-%: >>>> TESTS += semihosting semiconsole >>>> endif >>>> +test-plugin-mem-access: CFLAGS+=-pthread -O0 >>>> +test-plugin-mem-access: LDFLAGS+=-pthread -O0 >>>> + >>>> +ifeq ($(CONFIG_PLUGIN),y) >>>> # Test plugin memory access instrumentation >>>> run-plugin-test-plugin-mem-access-with-libmem.so: \ >>>> PLUGIN_ARGS=$(COMMA)print-accesses=true >>>> @@ -197,8 +201,8 @@ run-plugin-test-plugin-mem-access-with-libmem.so: \ >>>> $(SRC_PATH)/tests/tcg/multiarch/check-plugin-output.sh \ >>>> $(QEMU) $< >>>> -test-plugin-mem-access: CFLAGS+=-pthread -O0 >>>> -test-plugin-mem-access: LDFLAGS+=-pthread -O0 >>>> +EXTRA_RUNS += run-plugin-test-plugin-mem-access-with-libmem.so >>>> +endif >>>> # Update TESTS >>>> TESTS += $(MULTIARCH_TESTS) >>>> diff --git a/tests/tcg/multiarch/system/Makefile.softmmu-target b/tests/tcg/multiarch/system/Makefile.softmmu-target >>>> index 5acf2700812..4171b4e6aa0 100644 >>>> --- a/tests/tcg/multiarch/system/Makefile.softmmu-target >>>> +++ b/tests/tcg/multiarch/system/Makefile.softmmu-target >>>> @@ -71,8 +71,11 @@ endif >>>> MULTIARCH_RUNS += run-gdbstub-memory run-gdbstub-interrupt \ >>>> run-gdbstub-untimely-packet run-gdbstub-registers >>>> +ifeq ($(CONFIG_PLUGIN),y) >>>> # Test plugin memory access instrumentation >>>> -run-plugin-memory-with-libmem.so: \ >>>> - PLUGIN_ARGS=$(COMMA)region-summary=true >>>> -run-plugin-memory-with-libmem.so: \ >>>> - CHECK_PLUGIN_OUTPUT_COMMAND=$(MULTIARCH_SYSTEM_SRC)/validate-memory-counts.py $@.out >>>> +run-plugin-memory-with-libmem.so: memory libmem.so >>>> +run-plugin-memory-with-libmem.so: PLUGIN_ARGS=$(COMMA)region-summary=true >>>> +run-plugin-memory-with-libmem.so: CHECK_PLUGIN_OUTPUT_COMMAND=$(MULTIARCH_SYSTEM_SRC)/validate-memory-counts.py $@.out >>>> + >>>> +EXTRA_RUNS += run-plugin-memory-with-libmem.so >>>> +endif >>> >>> I'm not sure how it's related, but check-tcg on aarch64 host now fails [1] since this series was merged, and I suspect it may be related to this patch. I didn't spend time to reproduce and investigate it. >>> >>> [1] https://github.com/pbo-linaro/qemu-ci/actions/runs/16575679153/job/46879690693 >> >> I was not able to reproduce it locally on 22.04, but in the CI indeed the test command is missing the "test-plugin-mem-access" binary at the end, just before redirection to .so.out, it should be: >> >> [...] -D test-plugin-mem-access-with-libmem.so.pout test-plugin-mem-access > run-plugin-test-plugin-mem-access-with-libmem.so.out >> >> not: >> >> [...] -D test-plugin-mem-access-with-libmem.so.pout > run-plugin-test-plugin-mem-access-with-libmem.so.out > > The problem is that the new rules generated by a shuffled combination of tests and plugin > (the one using eval) sometimes does not include the rule for the test+plugin combination > passed via EXTRA_RUNs. > > So EXTRA_RUNS tests like run-plugin-test-plugin-mem-access-with-libmem.so might end up > with a proper rule if the test test-plugin-mem-access is combined with other plugin > randomly, since now a plugin is picked up based on a tests index _idx modulo with the > number of plugins. > > A possible fix is to generate correctly the rules for the tests with plugins passed > via EXTRA_RUNS and ideally, following the patch's mood, remove the test from running > if any other plugin (no shuffle with any other plugin). > > Of course, as Pierrick said, this patch could be a premature optimization. So maybe > it doesn´t justify adding more complexity to our Makefile (Makefiles are hard to debug, > with 'eval' tricks, it's _reallly_ hard, so the more we avoid it the better). > > That said, if we want to keep this commit, I've kicked off a test to fix it here: > > https://gitlab.com/gusbromero/qemu/-/pipelines/1959953122 > > and sent the fix to the ML too. HTH. > Thanks for taking a look at this Gustavo. In the pipeline you shared, the build-some-softmmu fails with: Makefile:210: *** multiple target patterns. Stop. make: *** [/builds/gusbromero/qemu/tests/Makefile.include:50: build-tcg-tests-x86_64-softmmu] Error 2 make: *** Waiting for unfinished jobs.... Is that a new bug related to current patch? > > Cheers, > Gustavo
Hi Stefan and Pierrick, On 7/31/25 16:23, Pierrick Bouvier wrote: > On 7/31/25 11:58 AM, Gustavo Romero wrote: >> Hi, >> >> On 7/28/25 17:14, Gustavo Romero wrote: >>> Hi, >>> >>> On 7/28/25 16:03, Pierrick Bouvier wrote: >>>> Hi Alex, >>>> >>>> On 7/27/25 1:32 AM, Alex Bennée wrote: >>>>> As our set of multiarch tests has grown the practice of running every >>>>> plugin with every test is becoming unsustainable. If we switch to >>>>> ensuring every test gets run with at least one plugin we can speed >>>>> things up. >>>>> >>>>> Some plugins do need to be run with specific tests (for example the >>>>> memory instrumentation test). We can handle this by manually adding >>>>> them to EXTRA_RUNS. We also need to wrap rules in a CONFIG_PLUGIN test >>>>> so we don't enable the runs when plugins are not enabled. >>>>> >>>>> Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org> >>>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >>>>> Message-ID: <20250725154517.3523095-12-alex.bennee@linaro.org> >>>>> >>>>> diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target >>>>> index a12b15637ea..18afd5be194 100644 >>>>> --- a/tests/tcg/Makefile.target >>>>> +++ b/tests/tcg/Makefile.target >>>>> @@ -173,14 +173,25 @@ PLUGINS=$(filter-out $(DISABLE_PLUGINS), \ >>>>> # We need to ensure expand the run-plugin-TEST-with-PLUGIN >>>>> # pre-requistes manually here as we can't use stems to handle it. We >>>>> # only expand MULTIARCH_TESTS which are common on most of our targets >>>>> -# to avoid an exponential explosion as new tests are added. We also >>>>> -# add some special helpers the run-plugin- rules can use below. >>>>> +# and rotate the plugins so we don't grow too out of control as new >>>>> +# tests are added. Plugins that need to run with a specific test >>>>> +# should ensure they add their combination to EXTRA_RUNS. >>>>> ifneq ($(MULTIARCH_TESTS),) >>>>> -$(foreach p,$(PLUGINS), \ >>>>> - $(foreach t,$(MULTIARCH_TESTS),\ >>>>> - $(eval run-plugin-$(t)-with-$(p): $t $p) \ >>>>> - $(eval RUN_TESTS+=run-plugin-$(t)-with-$(p)))) >>>>> + >>>>> +NUM_PLUGINS := $(words $(PLUGINS)) >>>>> +NUM_TESTS := $(words $(MULTIARCH_TESTS)) >>>>> + >>>>> +define mod_plus_one >>>>> + $(shell $(PYTHON) -c "print( ($(1) % $(2)) + 1 )") >>>>> +endef >>>>> + >>>>> +$(foreach _idx, $(shell seq 1 $(NUM_TESTS)), \ >>>>> + $(eval _test := $(word $(_idx), $(MULTIARCH_TESTS))) \ >>>>> + $(eval _plugin := $(word $(call mod_plus_one, $(_idx), $(NUM_PLUGINS)), $(PLUGINS))) \ >>>>> + $(eval run-plugin-$(_test)-with-$(_plugin): $(_test) $(_plugin)) \ >>>>> + $(eval RUN_TESTS+=run-plugin-$(_test)-with-$(_plugin))) >>>>> + >>>>> endif # MULTIARCH_TESTS >>>>> endif # CONFIG_PLUGIN >>>>> diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target >>>>> index bfdf7197a7b..38345ff8805 100644 >>>>> --- a/tests/tcg/multiarch/Makefile.target >>>>> +++ b/tests/tcg/multiarch/Makefile.target >>>>> @@ -189,6 +189,10 @@ run-plugin-semiconsole-with-%: >>>>> TESTS += semihosting semiconsole >>>>> endif >>>>> +test-plugin-mem-access: CFLAGS+=-pthread -O0 >>>>> +test-plugin-mem-access: LDFLAGS+=-pthread -O0 >>>>> + >>>>> +ifeq ($(CONFIG_PLUGIN),y) >>>>> # Test plugin memory access instrumentation >>>>> run-plugin-test-plugin-mem-access-with-libmem.so: \ >>>>> PLUGIN_ARGS=$(COMMA)print-accesses=true >>>>> @@ -197,8 +201,8 @@ run-plugin-test-plugin-mem-access-with-libmem.so: \ >>>>> $(SRC_PATH)/tests/tcg/multiarch/check-plugin-output.sh \ >>>>> $(QEMU) $< >>>>> -test-plugin-mem-access: CFLAGS+=-pthread -O0 >>>>> -test-plugin-mem-access: LDFLAGS+=-pthread -O0 >>>>> +EXTRA_RUNS += run-plugin-test-plugin-mem-access-with-libmem.so >>>>> +endif >>>>> # Update TESTS >>>>> TESTS += $(MULTIARCH_TESTS) >>>>> diff --git a/tests/tcg/multiarch/system/Makefile.softmmu-target b/tests/tcg/multiarch/system/Makefile.softmmu-target >>>>> index 5acf2700812..4171b4e6aa0 100644 >>>>> --- a/tests/tcg/multiarch/system/Makefile.softmmu-target >>>>> +++ b/tests/tcg/multiarch/system/Makefile.softmmu-target >>>>> @@ -71,8 +71,11 @@ endif >>>>> MULTIARCH_RUNS += run-gdbstub-memory run-gdbstub-interrupt \ >>>>> run-gdbstub-untimely-packet run-gdbstub-registers >>>>> +ifeq ($(CONFIG_PLUGIN),y) >>>>> # Test plugin memory access instrumentation >>>>> -run-plugin-memory-with-libmem.so: \ >>>>> - PLUGIN_ARGS=$(COMMA)region-summary=true >>>>> -run-plugin-memory-with-libmem.so: \ >>>>> - CHECK_PLUGIN_OUTPUT_COMMAND=$(MULTIARCH_SYSTEM_SRC)/validate-memory-counts.py $@.out >>>>> +run-plugin-memory-with-libmem.so: memory libmem.so >>>>> +run-plugin-memory-with-libmem.so: PLUGIN_ARGS=$(COMMA)region-summary=true >>>>> +run-plugin-memory-with-libmem.so: CHECK_PLUGIN_OUTPUT_COMMAND=$(MULTIARCH_SYSTEM_SRC)/validate-memory-counts.py $@.out >>>>> + >>>>> +EXTRA_RUNS += run-plugin-memory-with-libmem.so >>>>> +endif >>>> >>>> I'm not sure how it's related, but check-tcg on aarch64 host now fails [1] since this series was merged, and I suspect it may be related to this patch. I didn't spend time to reproduce and investigate it. >>>> >>>> [1] https://github.com/pbo-linaro/qemu-ci/actions/runs/16575679153/job/46879690693 >>> >>> I was not able to reproduce it locally on 22.04, but in the CI indeed the test command is missing the "test-plugin-mem-access" binary at the end, just before redirection to .so.out, it should be: >>> >>> [...] -D test-plugin-mem-access-with-libmem.so.pout test-plugin-mem-access > run-plugin-test-plugin-mem-access-with-libmem.so.out >>> >>> not: >>> >>> [...] -D test-plugin-mem-access-with-libmem.so.pout > run-plugin-test-plugin-mem-access-with-libmem.so.out >> >> The problem is that the new rules generated by a shuffled combination of tests and plugin >> (the one using eval) sometimes does not include the rule for the test+plugin combination >> passed via EXTRA_RUNs. >> >> So EXTRA_RUNS tests like run-plugin-test-plugin-mem-access-with-libmem.so might end up >> with a proper rule if the test test-plugin-mem-access is combined with other plugin >> randomly, since now a plugin is picked up based on a tests index _idx modulo with the >> number of plugins. >> >> A possible fix is to generate correctly the rules for the tests with plugins passed >> via EXTRA_RUNS and ideally, following the patch's mood, remove the test from running >> if any other plugin (no shuffle with any other plugin). >> >> Of course, as Pierrick said, this patch could be a premature optimization. So maybe >> it doesn´t justify adding more complexity to our Makefile (Makefiles are hard to debug, >> with 'eval' tricks, it's _reallly_ hard, so the more we avoid it the better). >> >> That said, if we want to keep this commit, I've kicked off a test to fix it here: >> >> https://gitlab.com/gusbromero/qemu/-/pipelines/1959953122 >> >> and sent the fix to the ML too. HTH. >> > > Thanks for taking a look at this Gustavo. > > In the pipeline you shared, the build-some-softmmu fails with: > Makefile:210: *** multiple target patterns. Stop. > make: *** [/builds/gusbromero/qemu/tests/Makefile.include:50: build-tcg-tests-x86_64-softmmu] Error 2 > make: *** Waiting for unfinished jobs.... > > Is that a new bug related to current patch? v2 was busted. Please consider this fix, which passed the test pipeline: https://gitlab.com/gusbromero/qemu/-/pipelines/1960296034 https://mail.gnu.org/archive/html/qemu-devel/2025-07/msg06926.html Cheers, Gustavo
On 7/31/25 21:29, Gustavo Romero wrote: > Hi Stefan and Pierrick, > > On 7/31/25 16:23, Pierrick Bouvier wrote: >> On 7/31/25 11:58 AM, Gustavo Romero wrote: >>> Hi, >>> >>> On 7/28/25 17:14, Gustavo Romero wrote: >>>> Hi, >>>> >>>> On 7/28/25 16:03, Pierrick Bouvier wrote: >>>>> Hi Alex, >>>>> >>>>> On 7/27/25 1:32 AM, Alex Bennée wrote: >>>>>> As our set of multiarch tests has grown the practice of running every >>>>>> plugin with every test is becoming unsustainable. If we switch to >>>>>> ensuring every test gets run with at least one plugin we can speed >>>>>> things up. >>>>>> >>>>>> Some plugins do need to be run with specific tests (for example the >>>>>> memory instrumentation test). We can handle this by manually adding >>>>>> them to EXTRA_RUNS. We also need to wrap rules in a CONFIG_PLUGIN test >>>>>> so we don't enable the runs when plugins are not enabled. >>>>>> >>>>>> Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org> >>>>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >>>>>> Message-ID: <20250725154517.3523095-12-alex.bennee@linaro.org> >>>>>> >>>>>> diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target >>>>>> index a12b15637ea..18afd5be194 100644 >>>>>> --- a/tests/tcg/Makefile.target >>>>>> +++ b/tests/tcg/Makefile.target >>>>>> @@ -173,14 +173,25 @@ PLUGINS=$(filter-out $(DISABLE_PLUGINS), \ >>>>>> # We need to ensure expand the run-plugin-TEST-with-PLUGIN >>>>>> # pre-requistes manually here as we can't use stems to handle it. We >>>>>> # only expand MULTIARCH_TESTS which are common on most of our targets >>>>>> -# to avoid an exponential explosion as new tests are added. We also >>>>>> -# add some special helpers the run-plugin- rules can use below. >>>>>> +# and rotate the plugins so we don't grow too out of control as new >>>>>> +# tests are added. Plugins that need to run with a specific test >>>>>> +# should ensure they add their combination to EXTRA_RUNS. >>>>>> ifneq ($(MULTIARCH_TESTS),) >>>>>> -$(foreach p,$(PLUGINS), \ >>>>>> - $(foreach t,$(MULTIARCH_TESTS),\ >>>>>> - $(eval run-plugin-$(t)-with-$(p): $t $p) \ >>>>>> - $(eval RUN_TESTS+=run-plugin-$(t)-with-$(p)))) >>>>>> + >>>>>> +NUM_PLUGINS := $(words $(PLUGINS)) >>>>>> +NUM_TESTS := $(words $(MULTIARCH_TESTS)) >>>>>> + >>>>>> +define mod_plus_one >>>>>> + $(shell $(PYTHON) -c "print( ($(1) % $(2)) + 1 )") >>>>>> +endef >>>>>> + >>>>>> +$(foreach _idx, $(shell seq 1 $(NUM_TESTS)), \ >>>>>> + $(eval _test := $(word $(_idx), $(MULTIARCH_TESTS))) \ >>>>>> + $(eval _plugin := $(word $(call mod_plus_one, $(_idx), $(NUM_PLUGINS)), $(PLUGINS))) \ >>>>>> + $(eval run-plugin-$(_test)-with-$(_plugin): $(_test) $(_plugin)) \ >>>>>> + $(eval RUN_TESTS+=run-plugin-$(_test)-with-$(_plugin))) >>>>>> + >>>>>> endif # MULTIARCH_TESTS >>>>>> endif # CONFIG_PLUGIN >>>>>> diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target >>>>>> index bfdf7197a7b..38345ff8805 100644 >>>>>> --- a/tests/tcg/multiarch/Makefile.target >>>>>> +++ b/tests/tcg/multiarch/Makefile.target >>>>>> @@ -189,6 +189,10 @@ run-plugin-semiconsole-with-%: >>>>>> TESTS += semihosting semiconsole >>>>>> endif >>>>>> +test-plugin-mem-access: CFLAGS+=-pthread -O0 >>>>>> +test-plugin-mem-access: LDFLAGS+=-pthread -O0 >>>>>> + >>>>>> +ifeq ($(CONFIG_PLUGIN),y) >>>>>> # Test plugin memory access instrumentation >>>>>> run-plugin-test-plugin-mem-access-with-libmem.so: \ >>>>>> PLUGIN_ARGS=$(COMMA)print-accesses=true >>>>>> @@ -197,8 +201,8 @@ run-plugin-test-plugin-mem-access-with-libmem.so: \ >>>>>> $(SRC_PATH)/tests/tcg/multiarch/check-plugin-output.sh \ >>>>>> $(QEMU) $< >>>>>> -test-plugin-mem-access: CFLAGS+=-pthread -O0 >>>>>> -test-plugin-mem-access: LDFLAGS+=-pthread -O0 >>>>>> +EXTRA_RUNS += run-plugin-test-plugin-mem-access-with-libmem.so >>>>>> +endif >>>>>> # Update TESTS >>>>>> TESTS += $(MULTIARCH_TESTS) >>>>>> diff --git a/tests/tcg/multiarch/system/Makefile.softmmu-target b/tests/tcg/multiarch/system/Makefile.softmmu-target >>>>>> index 5acf2700812..4171b4e6aa0 100644 >>>>>> --- a/tests/tcg/multiarch/system/Makefile.softmmu-target >>>>>> +++ b/tests/tcg/multiarch/system/Makefile.softmmu-target >>>>>> @@ -71,8 +71,11 @@ endif >>>>>> MULTIARCH_RUNS += run-gdbstub-memory run-gdbstub-interrupt \ >>>>>> run-gdbstub-untimely-packet run-gdbstub-registers >>>>>> +ifeq ($(CONFIG_PLUGIN),y) >>>>>> # Test plugin memory access instrumentation >>>>>> -run-plugin-memory-with-libmem.so: \ >>>>>> - PLUGIN_ARGS=$(COMMA)region-summary=true >>>>>> -run-plugin-memory-with-libmem.so: \ >>>>>> - CHECK_PLUGIN_OUTPUT_COMMAND=$(MULTIARCH_SYSTEM_SRC)/validate-memory-counts.py $@.out >>>>>> +run-plugin-memory-with-libmem.so: memory libmem.so >>>>>> +run-plugin-memory-with-libmem.so: PLUGIN_ARGS=$(COMMA)region-summary=true >>>>>> +run-plugin-memory-with-libmem.so: CHECK_PLUGIN_OUTPUT_COMMAND=$(MULTIARCH_SYSTEM_SRC)/validate-memory-counts.py $@.out >>>>>> + >>>>>> +EXTRA_RUNS += run-plugin-memory-with-libmem.so >>>>>> +endif >>>>> >>>>> I'm not sure how it's related, but check-tcg on aarch64 host now fails [1] since this series was merged, and I suspect it may be related to this patch. I didn't spend time to reproduce and investigate it. >>>>> >>>>> [1] https://github.com/pbo-linaro/qemu-ci/actions/runs/16575679153/job/46879690693 >>>> >>>> I was not able to reproduce it locally on 22.04, but in the CI indeed the test command is missing the "test-plugin-mem-access" binary at the end, just before redirection to .so.out, it should be: >>>> >>>> [...] -D test-plugin-mem-access-with-libmem.so.pout test-plugin-mem-access > run-plugin-test-plugin-mem-access-with-libmem.so.out >>>> >>>> not: >>>> >>>> [...] -D test-plugin-mem-access-with-libmem.so.pout > run-plugin-test-plugin-mem-access-with-libmem.so.out >>> >>> The problem is that the new rules generated by a shuffled combination of tests and plugin >>> (the one using eval) sometimes does not include the rule for the test+plugin combination >>> passed via EXTRA_RUNs. >>> >>> So EXTRA_RUNS tests like run-plugin-test-plugin-mem-access-with-libmem.so might end up >>> with a proper rule if the test test-plugin-mem-access is combined with other plugin >>> randomly, since now a plugin is picked up based on a tests index _idx modulo with the >>> number of plugins. >>> >>> A possible fix is to generate correctly the rules for the tests with plugins passed >>> via EXTRA_RUNS and ideally, following the patch's mood, remove the test from running >>> if any other plugin (no shuffle with any other plugin). >>> >>> Of course, as Pierrick said, this patch could be a premature optimization. So maybe >>> it doesn´t justify adding more complexity to our Makefile (Makefiles are hard to debug, >>> with 'eval' tricks, it's _reallly_ hard, so the more we avoid it the better). >>> >>> That said, if we want to keep this commit, I've kicked off a test to fix it here: >>> >>> https://gitlab.com/gusbromero/qemu/-/pipelines/1959953122 >>> >>> and sent the fix to the ML too. HTH. >>> >> >> Thanks for taking a look at this Gustavo. >> >> In the pipeline you shared, the build-some-softmmu fails with: >> Makefile:210: *** multiple target patterns. Stop. >> make: *** [/builds/gusbromero/qemu/tests/Makefile.include:50: build-tcg-tests-x86_64-softmmu] Error 2 >> make: *** Waiting for unfinished jobs.... >> >> Is that a new bug related to current patch? > > v2 was busted. Please consider this fix, which passed the test pipeline: v1, I meant. Please consider v2 as the good one :) > https://gitlab.com/gusbromero/qemu/-/pipelines/1960296034 > > https://mail.gnu.org/archive/html/qemu-devel/2025-07/msg06926.html Cheers, Gustavo
Hi Pierrick, On 7/31/25 16:23, Pierrick Bouvier wrote: > On 7/31/25 11:58 AM, Gustavo Romero wrote: >> Hi, >> >> On 7/28/25 17:14, Gustavo Romero wrote: >>> Hi, >>> >>> On 7/28/25 16:03, Pierrick Bouvier wrote: >>>> Hi Alex, >>>> >>>> On 7/27/25 1:32 AM, Alex Bennée wrote: >>>>> As our set of multiarch tests has grown the practice of running every >>>>> plugin with every test is becoming unsustainable. If we switch to >>>>> ensuring every test gets run with at least one plugin we can speed >>>>> things up. >>>>> >>>>> Some plugins do need to be run with specific tests (for example the >>>>> memory instrumentation test). We can handle this by manually adding >>>>> them to EXTRA_RUNS. We also need to wrap rules in a CONFIG_PLUGIN test >>>>> so we don't enable the runs when plugins are not enabled. >>>>> >>>>> Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org> >>>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >>>>> Message-ID: <20250725154517.3523095-12-alex.bennee@linaro.org> >>>>> >>>>> diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target >>>>> index a12b15637ea..18afd5be194 100644 >>>>> --- a/tests/tcg/Makefile.target >>>>> +++ b/tests/tcg/Makefile.target >>>>> @@ -173,14 +173,25 @@ PLUGINS=$(filter-out $(DISABLE_PLUGINS), \ >>>>> # We need to ensure expand the run-plugin-TEST-with-PLUGIN >>>>> # pre-requistes manually here as we can't use stems to handle it. We >>>>> # only expand MULTIARCH_TESTS which are common on most of our targets >>>>> -# to avoid an exponential explosion as new tests are added. We also >>>>> -# add some special helpers the run-plugin- rules can use below. >>>>> +# and rotate the plugins so we don't grow too out of control as new >>>>> +# tests are added. Plugins that need to run with a specific test >>>>> +# should ensure they add their combination to EXTRA_RUNS. >>>>> ifneq ($(MULTIARCH_TESTS),) >>>>> -$(foreach p,$(PLUGINS), \ >>>>> - $(foreach t,$(MULTIARCH_TESTS),\ >>>>> - $(eval run-plugin-$(t)-with-$(p): $t $p) \ >>>>> - $(eval RUN_TESTS+=run-plugin-$(t)-with-$(p)))) >>>>> + >>>>> +NUM_PLUGINS := $(words $(PLUGINS)) >>>>> +NUM_TESTS := $(words $(MULTIARCH_TESTS)) >>>>> + >>>>> +define mod_plus_one >>>>> + $(shell $(PYTHON) -c "print( ($(1) % $(2)) + 1 )") >>>>> +endef >>>>> + >>>>> +$(foreach _idx, $(shell seq 1 $(NUM_TESTS)), \ >>>>> + $(eval _test := $(word $(_idx), $(MULTIARCH_TESTS))) \ >>>>> + $(eval _plugin := $(word $(call mod_plus_one, $(_idx), $(NUM_PLUGINS)), $(PLUGINS))) \ >>>>> + $(eval run-plugin-$(_test)-with-$(_plugin): $(_test) $(_plugin)) \ >>>>> + $(eval RUN_TESTS+=run-plugin-$(_test)-with-$(_plugin))) >>>>> + >>>>> endif # MULTIARCH_TESTS >>>>> endif # CONFIG_PLUGIN >>>>> diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target >>>>> index bfdf7197a7b..38345ff8805 100644 >>>>> --- a/tests/tcg/multiarch/Makefile.target >>>>> +++ b/tests/tcg/multiarch/Makefile.target >>>>> @@ -189,6 +189,10 @@ run-plugin-semiconsole-with-%: >>>>> TESTS += semihosting semiconsole >>>>> endif >>>>> +test-plugin-mem-access: CFLAGS+=-pthread -O0 >>>>> +test-plugin-mem-access: LDFLAGS+=-pthread -O0 >>>>> + >>>>> +ifeq ($(CONFIG_PLUGIN),y) >>>>> # Test plugin memory access instrumentation >>>>> run-plugin-test-plugin-mem-access-with-libmem.so: \ >>>>> PLUGIN_ARGS=$(COMMA)print-accesses=true >>>>> @@ -197,8 +201,8 @@ run-plugin-test-plugin-mem-access-with-libmem.so: \ >>>>> $(SRC_PATH)/tests/tcg/multiarch/check-plugin-output.sh \ >>>>> $(QEMU) $< >>>>> -test-plugin-mem-access: CFLAGS+=-pthread -O0 >>>>> -test-plugin-mem-access: LDFLAGS+=-pthread -O0 >>>>> +EXTRA_RUNS += run-plugin-test-plugin-mem-access-with-libmem.so >>>>> +endif >>>>> # Update TESTS >>>>> TESTS += $(MULTIARCH_TESTS) >>>>> diff --git a/tests/tcg/multiarch/system/Makefile.softmmu-target b/tests/tcg/multiarch/system/Makefile.softmmu-target >>>>> index 5acf2700812..4171b4e6aa0 100644 >>>>> --- a/tests/tcg/multiarch/system/Makefile.softmmu-target >>>>> +++ b/tests/tcg/multiarch/system/Makefile.softmmu-target >>>>> @@ -71,8 +71,11 @@ endif >>>>> MULTIARCH_RUNS += run-gdbstub-memory run-gdbstub-interrupt \ >>>>> run-gdbstub-untimely-packet run-gdbstub-registers >>>>> +ifeq ($(CONFIG_PLUGIN),y) >>>>> # Test plugin memory access instrumentation >>>>> -run-plugin-memory-with-libmem.so: \ >>>>> - PLUGIN_ARGS=$(COMMA)region-summary=true >>>>> -run-plugin-memory-with-libmem.so: \ >>>>> - CHECK_PLUGIN_OUTPUT_COMMAND=$(MULTIARCH_SYSTEM_SRC)/validate-memory-counts.py $@.out >>>>> +run-plugin-memory-with-libmem.so: memory libmem.so >>>>> +run-plugin-memory-with-libmem.so: PLUGIN_ARGS=$(COMMA)region-summary=true >>>>> +run-plugin-memory-with-libmem.so: CHECK_PLUGIN_OUTPUT_COMMAND=$(MULTIARCH_SYSTEM_SRC)/validate-memory-counts.py $@.out >>>>> + >>>>> +EXTRA_RUNS += run-plugin-memory-with-libmem.so >>>>> +endif >>>> >>>> I'm not sure how it's related, but check-tcg on aarch64 host now fails [1] since this series was merged, and I suspect it may be related to this patch. I didn't spend time to reproduce and investigate it. >>>> >>>> [1] https://github.com/pbo-linaro/qemu-ci/actions/runs/16575679153/job/46879690693 >>> >>> I was not able to reproduce it locally on 22.04, but in the CI indeed the test command is missing the "test-plugin-mem-access" binary at the end, just before redirection to .so.out, it should be: >>> >>> [...] -D test-plugin-mem-access-with-libmem.so.pout test-plugin-mem-access > run-plugin-test-plugin-mem-access-with-libmem.so.out >>> >>> not: >>> >>> [...] -D test-plugin-mem-access-with-libmem.so.pout > run-plugin-test-plugin-mem-access-with-libmem.so.out >> >> The problem is that the new rules generated by a shuffled combination of tests and plugin >> (the one using eval) sometimes does not include the rule for the test+plugin combination >> passed via EXTRA_RUNs. >> >> So EXTRA_RUNS tests like run-plugin-test-plugin-mem-access-with-libmem.so might end up >> with a proper rule if the test test-plugin-mem-access is combined with other plugin >> randomly, since now a plugin is picked up based on a tests index _idx modulo with the >> number of plugins. >> >> A possible fix is to generate correctly the rules for the tests with plugins passed >> via EXTRA_RUNS and ideally, following the patch's mood, remove the test from running >> if any other plugin (no shuffle with any other plugin). >> >> Of course, as Pierrick said, this patch could be a premature optimization. So maybe >> it doesn´t justify adding more complexity to our Makefile (Makefiles are hard to debug, >> with 'eval' tricks, it's _reallly_ hard, so the more we avoid it the better). >> >> That said, if we want to keep this commit, I've kicked off a test to fix it here: >> >> https://gitlab.com/gusbromero/qemu/-/pipelines/1959953122 >> >> and sent the fix to the ML too. HTH. >> > > Thanks for taking a look at this Gustavo. > > In the pipeline you shared, the build-some-softmmu fails with: > Makefile:210: *** multiple target patterns. Stop. > make: *** [/builds/gusbromero/qemu/tests/Makefile.include:50: build-tcg-tests-x86_64-softmmu] Error 2 > make: *** Waiting for unfinished jobs.... > > Is that a new bug related to current patch? Yeah. I'm taking a look. I'll get back in a bit. Thanks. Cheers, Gustavo >> >> Cheers, >> Gustavo >
© 2016 - 2025 Red Hat, Inc.