[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAGudoHGBedD35u9FnYyPuJV=vT9mUrbtRVREO1P0RdzHhV=1FQ@mail.gmail.com>
Date: Wed, 17 Sep 2025 07:22:46 +0200
From: Mateusz Guzik <mjguzik@...il.com>
To: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Cc: Chris Mason <clm@...a.com>, Andrew Morton <akpm@...ux-foundation.org>,
Gerald Schaefer <gerald.schaefer@...ux.ibm.com>, Heiko Carstens <hca@...ux.ibm.com>,
Vasily Gorbik <gor@...ux.ibm.com>, Christian Borntraeger <borntraeger@...ux.ibm.com>,
Sven Schnelle <svens@...ux.ibm.com>, "David S . Miller" <davem@...emloft.net>,
Andreas Larsson <andreas@...sler.com>, Dave Hansen <dave.hansen@...ux.intel.com>,
Andy Lutomirski <luto@...nel.org>, Peter Zijlstra <peterz@...radead.org>,
Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
"H . Peter Anvin" <hpa@...or.com>, Alexander Viro <viro@...iv.linux.org.uk>,
Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>, Kees Cook <kees@...nel.org>,
David Hildenbrand <david@...hat.com>, Zi Yan <ziy@...dia.com>,
Baolin Wang <baolin.wang@...ux.alibaba.com>,
"Liam R . Howlett" <Liam.Howlett@...cle.com>, Nico Pache <npache@...hat.com>,
Ryan Roberts <ryan.roberts@....com>, Dev Jain <dev.jain@....com>, Barry Song <baohua@...nel.org>,
Xu Xin <xu.xin16@....com.cn>, Chengming Zhou <chengming.zhou@...ux.dev>,
Vlastimil Babka <vbabka@...e.cz>, Mike Rapoport <rppt@...nel.org>, Suren Baghdasaryan <surenb@...gle.com>,
Michal Hocko <mhocko@...e.com>, David Rientjes <rientjes@...gle.com>,
Shakeel Butt <shakeel.butt@...ux.dev>, Arnaldo Carvalho de Melo <acme@...nel.org>,
Namhyung Kim <namhyung@...nel.org>, Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>,
Ian Rogers <irogers@...gle.com>, Adrian Hunter <adrian.hunter@...el.com>,
Kan Liang <kan.liang@...ux.intel.com>, Masami Hiramatsu <mhiramat@...nel.org>,
Oleg Nesterov <oleg@...hat.com>, Juri Lelli <juri.lelli@...hat.com>,
Vincent Guittot <vincent.guittot@...aro.org>, Dietmar Eggemann <dietmar.eggemann@....com>,
Steven Rostedt <rostedt@...dmis.org>, Ben Segall <bsegall@...gle.com>, Mel Gorman <mgorman@...e.de>,
Valentin Schneider <vschneid@...hat.com>, Jason Gunthorpe <jgg@...pe.ca>, John Hubbard <jhubbard@...dia.com>,
Peter Xu <peterx@...hat.com>, Jann Horn <jannh@...gle.com>, Pedro Falcato <pfalcato@...e.de>,
Matthew Wilcox <willy@...radead.org>, linux-s390@...r.kernel.org,
linux-kernel@...r.kernel.org, sparclinux@...r.kernel.org,
linux-fsdevel@...r.kernel.org, linux-mm@...ck.org,
linux-trace-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org
Subject: Re: [PATCH 02/10] mm: convert core mm to mm_flags_*() accessors
On Wed, Sep 17, 2025 at 7:20 AM Lorenzo Stoakes
<lorenzo.stoakes@...cle.com> wrote:
>
> On Wed, Sep 17, 2025 at 02:16:54AM +0200, Mateusz Guzik wrote:
> > On Wed, Sep 17, 2025 at 1:57 AM Chris Mason <clm@...a.com> wrote:
> > >
> > > On Tue, 12 Aug 2025 16:44:11 +0100 Lorenzo Stoakes <lorenzo.stoakes@...cle.com> wrote:
> > >
> > > > 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 <lorenzo.stoakes@...cle.com>
> > >
> > > [ ... ]
> > >
> > > > 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
> > >
> > > [ ... ]
> > >
> > > > @@ -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
> >
> > 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
>
> >
> > $ 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);
Powered by blists - more mailing lists