Re: [PATCH 02/10] mm: convert core mm to mm_flags_*() accessors
From: Lorenzo Stoakes <hidden>
Date: 2025-09-17 05:21:20
Also in:
linux-fsdevel, linux-mm, linux-perf-users, linux-s390, lkml, sparclinux
On Wed, Sep 17, 2025 at 02:16:54AM +0200, Mateusz Guzik wrote:
On Wed, Sep 17, 2025 at 1:57 AM Chris Mason [off-list ref] wrote:quoted
On Tue, 12 Aug 2025 16:44:11 +0100 Lorenzo Stoakes [off-list ref] wrote:quoted
As part of the effort to move to mm->flags becoming a bitmap field, convert existing users to making use of the mm_flags_*() accessors which will, when the conversion is complete, be the only means of accessing mm_struct flags. This will result in the debug output being that of a bitmap output, which will result in a minor change here, but since this is for debug only, this should have no bearing. Otherwise, no functional changes intended. Signed-off-by: Lorenzo Stoakes <redacted>[ ... ]quoted
diff --git a/mm/oom_kill.c b/mm/oom_kill.c index 25923cfec9c6..17650f0b516e 100644 --- a/mm/oom_kill.c +++ b/mm/oom_kill.c[ ... ]quoted
@@ -1251,7 +1251,7 @@ SYSCALL_DEFINE2(process_mrelease, int, pidfd, unsigned int, flags) * Check MMF_OOM_SKIP again under mmap_read_lock protection to ensure * possible change in exit_mmap is seen */ - if (!test_bit(MMF_OOM_SKIP, &mm->flags) && !__oom_reap_task_mm(mm)) + if (mm_flags_test(MMF_OOM_SKIP, mm) && !__oom_reap_task_mm(mm)) ret = -EAGAIN; mmap_read_unlock(mm);Hi Lorzeno, I think we lost a ! here. claude found enough inverted logic in moved code that I did a new run with a more explicit prompt for it, but this was the only new hit.I presume conversion was done mostly manually?
Actually largely via sed/emacs find-replace. I'm not sure why this case happened. But maybe it's one of the not 'largely' changes... Human-in-the-middle is obviously subject to errors :)
The way(tm) is to use coccinelle. I whipped out the following real quick and results look good: @@ expression mm, bit; @@ - test_bit(bit, &mm->flags) + mm_flags_test(bit, mm)
Thanks. Not sure it'd hit every case. But that's useful to know, could presumably expand to hit others. I will be changing VMA flags when my review load finally allows me to so knowing this is useful... Cheers, Lorenzo
quoted hunk ↗ jump to hunk
$ spatch --sp-file mmbit.cocci mm/oom_kill.c [snip]@@ -892,7 +892,7 @@ static bool task_will_free_mem(struct ta * This task has already been drained by the oom reaper so there are * only small chances it will free some more */ - if (test_bit(MMF_OOM_SKIP, &mm->flags)) + if (mm_flags_test(MMF_OOM_SKIP, mm)) return false; if (atomic_read(&mm->mm_users) <= 1)@@ -1235,7 +1235,7 @@ SYSCALL_DEFINE2(process_mrelease, int, p reap = true; else { /* Error only if the work has not been done already */ - if (!test_bit(MMF_OOM_SKIP, &mm->flags)) + if (!mm_flags_test(MMF_OOM_SKIP, mm)) ret = -EINVAL; } task_unlock(p);@@ -1251,7 +1251,7 @@ SYSCALL_DEFINE2(process_mrelease, int, p * Check MMF_OOM_SKIP again under mmap_read_lock protection to ensure * possible change in exit_mmap is seen */ - if (!test_bit(MMF_OOM_SKIP, &mm->flags) && !__oom_reap_task_mm(mm)) + if (!mm_flags_test(MMF_OOM_SKIP, mm) && !__oom_reap_task_mm(mm)) ret = -EAGAIN; mmap_read_unlock(mm);