Re: [PATCH v4 13/15] selftests/mm: move hwpoison setup into run_test() and silence modprobe output for memory-failure category

[Zi Yan <ziy@nvidia.com>]

On 8 Apr 2026, at 4:36, Sayali Patil wrote:

> On 07/04/26 20:21, Zi Yan wrote:
>> On 6 Apr 2026, at 5:19, Sayali Patil wrote:
>>
>>> run_vmtests.sh contains special handling to ensure the hwpoison_inject
>>> module is available for the memory-failure tests. This logic was
>>> implemented outside of run_test(), making the setup category-specific
>>> but managed globally.
>>>
>>> Move the hwpoison_inject handling into run_test() and restrict it
>>> to the memory-failure category so that:
>>> 1. the module is checked and loaded only when memory-failure tests run,
>>> 2. the test is skipped if the module or the debugfs interface
>>> (/sys/kernel/debug/hwpoison/) is not available.
>>> 3. the module is unloaded after the test if it was loaded by the script.
>>>
>>> This localizes category-specific setup and makes the test flow
>>> consistent with other per-category preparations.
>>>
>>> While updating this logic, fix the module availability check.
>>> The script previously used:
>>>
>>> 	modprobe -R hwpoison_inject
>>>
>>> The -R option prints the resolved module name to stdout, causing every
>>> run to print:
>>>
>>> 	hwpoison_inject
>>>
>>> in the test output, even when no action is required, introducing
>>> unnecessary noise.
>>>
>>> Replace this with:
>>>
>>> 	modprobe -n hwpoison_inject
>>>
>>> which verifies that the module is loadable without producing output,
>>> keeping the selftest logs clean and consistent.
>>>
>>> Also, ensure that skipped tests do not override a previously recorded
>>> failure. A skipped test currently sets exitcode to ksft_skip even if a
>>> prior test has failed, which can mask failures in the final exit status.
>>> Update the logic to only set exitcode to ksft_skip when no failure has
>>> been recorded.
>>>
>>> Fixes: ff4ef2fbd101 ("selftests/mm: add memory failure anonymous page test")
>>> Signed-off-by: Sayali Patil <sayalip@linux.ibm.com>
>>> ---
>>>   tools/testing/selftests/mm/run_vmtests.sh | 52 ++++++++++++++---------
>>>   1 file changed, 33 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
>>> index afdcfd0d7cef..17c9bd910c47 100755
>>> --- a/tools/testing/selftests/mm/run_vmtests.sh
>>> +++ b/tools/testing/selftests/mm/run_vmtests.sh
>>> @@ -235,6 +235,7 @@ pretty_name() {
>>>   run_test() {
>>>   	if test_selected ${CATEGORY}; then
>>>   		local skip=0
>>> +		local LOADED_MOD=0
>>
>> Can you rename it to LOADED_MEMORY_FAILURE_MOD to clarify its use?
>> Since now LOADED_MOD is visible for the entire run_test().
>
> Thanks for the review!
>
> I kept it as LOADED_MOD and specified it as a local variable intentionally. The idea was that, if any future tests are added that need to verify whether the module was loaded, they can reuse the same variable. For that reason, I did not make it specific to the memory failure test.

You mean future tests that use hwpoison_inject module? Or any future tests
that load a module? For the former, you can use LOADED_HWPOISON_INJECT_MOD;
for the latter, I am not sure how a single LOADED_MOD would work for
different modules and that probably would need new code for that case.

My point is that since LOADED_MOD is now in the scope of run_test(), it is
unclear which test uses it without reading the entire run_test(). Giving a
specific name improves readability.


Best Regards,
Yan, Zi