linux-kernel - Re: [PATCH] mm/page_alloc: simplify and cleanup pcp locking

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAJuCfpHDr95JnAkK_KzTU+VMLnHAuDVkFjPn4XnG4KpLtqEGLw@mail.gmail.com>
Date: Wed, 15 Oct 2025 11:50:38 -0700
From: Suren Baghdasaryan <surenb@...gle.com>
To: Vlastimil Babka <vbabka@...e.cz>
Cc: Joshua Hahn <joshua.hahnjy@...il.com>, Andrew Morton <akpm@...ux-foundation.org>, 
	Michal Hocko <mhocko@...e.com>, Brendan Jackman <jackmanb@...gle.com>, 
	Johannes Weiner <hannes@...xchg.org>, Zi Yan <ziy@...dia.com>, 
	Mel Gorman <mgorman@...hsingularity.net>, linux-mm@...ck.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm/page_alloc: simplify and cleanup pcp locking

On Wed, Oct 15, 2025 at 9:08 AM Vlastimil Babka <vbabka@...e.cz> wrote:
>
> On 10/15/25 16:51, Joshua Hahn wrote:
> > On Wed, 15 Oct 2025 11:36:09 +0200 Vlastimil Babka <vbabka@...e.cz> wrote:
> >
> >> The pcp locking relies on pcp_spin_trylock() which has to be used
> >> together with pcp_trylock_prepare()/pcp_trylock_finish() to work
> >> properly on !SMP !RT configs. This is tedious and error-prone.
> >>
> >> We can remove pcp_spin_lock() and underlying pcpu_spin_lock() because we
> >> don't use it. Afterwards pcpu_spin_unlock() is only used together with
> >> pcp_spin_trylock(). Therefore we can add the UP_flags parameter to them
> >> and handle pcp_trylock_prepare()/finish() within them.
> >>
> >> Additionally for the configs where pcp_trylock_prepare() is a no-op (SMP
> >> || RT) make it pass &UP_flags to a no-op inline function. This ensures
> >> typechecking and makes the local variable "used" so we can remove the
> >> __maybe_unused attributes.
> >>
> >> In my compile testing, bloat-o-meter reported no change on SMP config,
> >> so the compiler is capable of optimizing away the no-ops same as before,
> >> and we have simplified the code using pcp_spin_trylock().
> >>
> >> Signed-off-by: Vlastimil Babka <vbabka@...e.cz>
> >
> > Hello Vlastimil, I hope you are doing well!
> >
> > Thank you for this patch. This is a pattern that I found quite cumbersome,
> > so this patch really makes the code so much easier to understand and read.
>
> Hi, that's good to hear!
> >> ---
> >> based on mm-new
> >> ---
> >>  mm/page_alloc.c | 99 +++++++++++++++++++++++----------------------------------
> >>  1 file changed, 40 insertions(+), 59 deletions(-)
> >>
> >> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> >> index 0155a66d7367..2bf707f92d83 100644
> >> --- a/mm/page_alloc.c
> >> +++ b/mm/page_alloc.c
> >> @@ -99,9 +99,12 @@ static DEFINE_MUTEX(pcp_batch_high_lock);
> >>  /*
> >>   * On SMP, spin_trylock is sufficient protection.
> >>   * On PREEMPT_RT, spin_trylock is equivalent on both SMP and UP.
> >> + * Pass flags to a no-op inline function to typecheck and silence the unused
> >> + * variable warning.
> >>   */
> >> -#define pcp_trylock_prepare(flags)  do { } while (0)
> >> -#define pcp_trylock_finish(flag)    do { } while (0)
> >> +static inline void __pcp_trylock_prepare(unsigned long *flags) { }
> >> +#define pcp_trylock_prepare(flags) __pcp_trylock_prepare(&(flags))
> >> +#define pcp_trylock_finish(flags)   do { } while (0)
> >>  #else
> >
> > I have one question here. I was a bit unsure why we do the typechecking and
> > silencing for the unused variable warning for only pcp_trylock_prepare, but
> > not for pcp_trylock_finish. Is it because pcp_trylock_finish will always
> > be called after pcp_trylock_prepare, so the flag will have been used at
> > that point?
>
> Exactly.
>
> > I was concerned that there would have been some area where only
> > pcp_trylock_finish would have been used, but compiling with W=1 seems to show
> > no errors on my end : -) So it looks good to me! Feel free to add:
>
> Yeah we can change that if ever we end up with some code that needs it
> (hopefully not).
>
> > Reviewed-by: Joshua Hahn <joshua.hahnjy@...il.com>

Very nice!

Reviewed-by: Suren Baghdasaryan <surenb@...gle.com>

>
> Thanks!
>
> > Thank you! I hope you have a great day!
> > Joshua
>