Re: [PATCH v2 7/7] mm: switch deferred split shrinker to list_lru

[David Hildenbrand (Arm)]

On 3/20/26 17:02, Johannes Weiner wrote:
> On Thu, Mar 19, 2026 at 08:21:21AM +0100, David Hildenbrand (Arm) wrote:
>>>
>>> I remember you raising this in the objcg + reparenting patches. There
>>> are a few more instances of
>>>
>>> 	rcu_read_lock()
>>> 	foo = folio_memcg()
>>> 	...
>>> 	rcu_read_unlock()
>>>
>>> in other parts of the code not touched by these patches here, so the
>>> first pattern is a more universal encapsulation.
>>>
>>> Let me look into this. Would you be okay with a follow-up that covers
>>> the others as well?
>>
>> Of course :) If list_lru lock helpers would be the right thing to do, it
>> might be better placed in this series.
> 
> I'm playing around with the below. But there are a few things that
> seem suboptimal:

I like that as well (and could even be applied on top of the other
proposal later).

> 
> - We need a local @memcg, which makes sites that just pass
>   folio_memcg() somewhere else fatter. More site LOC on average.

The LOC is really mostly just from the helper functions IIUC.

> - Despite being more verbose, it communicates less. rcu_read_lock()
>   is universally understood, folio_memcg_foo() is cryptic.

begin/end is pretty clear IMHO. Not sure about the "cryptic" part. Taste
differs I guess. :)

> - It doesn't cover similar accessors with the same lifetime rules,
>   like folio_lruvec(), folio_memcg_check()


I think it gets inetresting once the RCU would implicitly protect other
stuff as well. And that would be my point: from the code alone it's not
quite clear what the RCU actually protects and what new code should be
using.

But I won't push for that if you/others prefer to spell out the RCU
stuff :) Thanks for playing with the code!


-- 
Cheers,

David