Re: [PATCH 02/10] mm: convert core mm to mm_flags_*() accessors

On Wed, Sep 17, 2025 at 7:20 AM Lorenzo Stoakes
[off-list ref] wrote:

On Wed, Sep 17, 2025 at 02:16:54AM +0200, Mateusz Guzik wrote:

quoted

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 :)

tru.dat

quoted

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...

I ran into bugs in spatch in the past where it just neglected to patch
something, but that's rare and can be trivially caught.

Defo easier to check than making sure none of the manual fixups are off.

Cheers, Lorenzo

quoted

$ 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);