public inbox for linux-mm@kvack.org
 help / color / mirror / Atom feed
From: Johannes Weiner <hannes@cmpxchg.org>
To: Shakeel Butt <shakeel.butt@linux.dev>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	David Hildenbrand <david@kernel.org>,
	Yosry Ahmed <yosry.ahmed@linux.dev>, Zi Yan <ziy@nvidia.com>,
	"Liam R. Howlett" <Liam.Howlett@oracle.com>,
	Usama Arif <usama.arif@linux.dev>,
	Kiryl Shutsemau <kas@kernel.org>,
	Dave Chinner <david@fromorbit.com>,
	Roman Gushchin <roman.gushchin@linux.dev>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 5/7] mm: list_lru: introduce caller locking for additions and deletions
Date: Fri, 20 Mar 2026 12:18:43 -0400	[thread overview]
Message-ID: <ab1zY5Ei6fPDfdpW@cmpxchg.org> (raw)
In-Reply-To: <absO1Bg_exH4llmH@linux.dev>

On Wed, Mar 18, 2026 at 01:51:04PM -0700, Shakeel Butt wrote:
> On Wed, Mar 18, 2026 at 03:53:23PM -0400, Johannes Weiner wrote:
> > Locking is currently internal to the list_lru API. However, a caller
> > might want to keep auxiliary state synchronized with the LRU state.
> > 
> > For example, the THP shrinker uses the lock of its custom LRU to keep
> > PG_partially_mapped and vmstats consistent.
> > 
> > To allow the THP shrinker to switch to list_lru, provide normal and
> > irqsafe locking primitives as well as caller-locked variants of the
> > addition and deletion functions.
> > 
> > Reviewed-by: David Hildenbrand (Arm) <david@kernel.org>
> > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> 
> One nit below, other than that:
> 
> Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
> 
> >  
> > -static inline void lock_list_lru(struct list_lru_one *l, bool irq)
> > +static inline void lock_list_lru(struct list_lru_one *l, bool irq,
> > +				 unsigned long *irq_flags)
> >  {
> > -	if (irq)
> > +	if (irq_flags)
> > +		spin_lock_irqsave(&l->lock, *irq_flags);
> > +	else if (irq)
> 
> If we move __list_lru_walk_one to use irq_flags then we can remove the irq
> param. It is reclaim code path and I don't think additional cost of irqsave
> would matter here.

The workingset shrinker's isolation function uses unlock_irq() and
cond_resched(). That would be non-trivial to rewrite - pass flags
around; keep irqs disabled for the whole reclaim cycle; break it into
a two-stage process. This sounds like a higher maintenance burden than
the bool here.

I know there is some cost to this distinction, but I actually do find
it useful to know the difference. It's self-documenting context.


  reply	other threads:[~2026-03-20 16:18 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-18 19:53 [PATCH v3 0/7] mm: switch THP shrinker to list_lru Johannes Weiner
2026-03-18 19:53 ` [PATCH v3 1/7] mm: list_lru: lock_list_lru_of_memcg() cannot return NULL if !skip_empty Johannes Weiner
2026-03-18 20:12   ` Shakeel Butt
2026-03-24 11:30   ` Lorenzo Stoakes (Oracle)
2026-03-18 19:53 ` [PATCH v3 2/7] mm: list_lru: deduplicate unlock_list_lru() Johannes Weiner
2026-03-24 11:32   ` Lorenzo Stoakes (Oracle)
2026-03-18 19:53 ` [PATCH v3 3/7] mm: list_lru: move list dead check to lock_list_lru_of_memcg() Johannes Weiner
2026-03-18 20:20   ` Shakeel Butt
2026-03-24 11:34   ` Lorenzo Stoakes (Oracle)
2026-03-18 19:53 ` [PATCH v3 4/7] mm: list_lru: deduplicate lock_list_lru() Johannes Weiner
2026-03-18 20:22   ` Shakeel Butt
2026-03-24 11:36   ` Lorenzo Stoakes (Oracle)
2026-03-18 19:53 ` [PATCH v3 5/7] mm: list_lru: introduce caller locking for additions and deletions Johannes Weiner
2026-03-18 20:51   ` Shakeel Butt
2026-03-20 16:18     ` Johannes Weiner [this message]
2026-03-24 11:55   ` Lorenzo Stoakes (Oracle)
2026-03-18 19:53 ` [PATCH v3 6/7] mm: list_lru: introduce folio_memcg_list_lru_alloc() Johannes Weiner
2026-03-18 20:52   ` Shakeel Butt
2026-03-18 21:01   ` Shakeel Butt
2026-03-24 12:01   ` Lorenzo Stoakes (Oracle)
2026-03-18 19:53 ` [PATCH v3 7/7] mm: switch deferred split shrinker to list_lru Johannes Weiner
2026-03-18 20:26   ` David Hildenbrand (Arm)
2026-03-18 23:18   ` Shakeel Butt
2026-03-24 13:48   ` Lorenzo Stoakes (Oracle)
2026-03-18 21:00 ` [PATCH v3 0/7] mm: switch THP " Lorenzo Stoakes (Oracle)
2026-03-18 22:31   ` Johannes Weiner
2026-03-19  8:47     ` Lorenzo Stoakes (Oracle)
2026-03-19  8:52       ` David Hildenbrand (Arm)
2026-03-19 11:45         ` Lorenzo Stoakes (Oracle)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ab1zY5Ei6fPDfdpW@cmpxchg.org \
    --to=hannes@cmpxchg.org \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@fromorbit.com \
    --cc=david@kernel.org \
    --cc=kas@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=roman.gushchin@linux.dev \
    --cc=shakeel.butt@linux.dev \
    --cc=usama.arif@linux.dev \
    --cc=yosry.ahmed@linux.dev \
    --cc=ziy@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox