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 <[email protected]>
>>> ---
>>> 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
