Re: [PATCH] mm/list_lru: Account node removals in isolation helpers

[Kaitao Cheng]

在 2026/11/6 22:06, Johannes Weiner 写道:
> On Wed, Nov 04, 2026 at 07:47:09AM +1100, Dave Chinner wrote:
>> On Tue, Nov 03, 2026 at 12:50:53PM +0800, Kaitao Cheng wrote:
>>> From: Kaitao Cheng <chengkaitao@xxxxxxxxxx>
>>>
>>> Successful writeback calls zswap_entry_free(), which removes the entry
>>> from the LRU and decrements both the sublist and node item counts.
>>> shrink_memcg_cb() then returns LRU_REMOVED_RETRY, causing the LRU walker
>>> to decrement the node count again.
>>>
>>> The affected call path before this change is:
>>>
>>>   __list_lru_walk_one()
>>>     shrink_memcg_cb()
>>>       zswap_writeback_entry()
>>>         zswap_entry_free()
>>>           zswap_lru_del()
>>>             list_lru_del()
>>>               __list_lru_del()
>>>                 atomic_long_dec()  /* first node count decrement */
>>>       return LRU_REMOVED_RETRY
>>>     atomic_long_dec()              /* second node count decrement */
>>
>> That's a bug in the zswap code. You shouldn't be calling
>> list_lru_del() from __list_lru_walk_one() context.
> 
> Grr, Kaitao you dropped the wrong thing that I thought had merit in my
> proposal...
> 
> Dave, would you mind taking a look at my patch at
> https://lore.kernel.org/all/auJkvb-Lq0M5Enkz@xxxxxxxxxxx/?
> 
> Ignore zswap for a second. I know it has problems. Most of it stems
> from the fact that we rely on the swap locking and do none of our own
> locking/lifetime on the zswap entry. As a result we work around it in
> odd ways. Sorry, you didn't ask :-) That's worth revisiting, although
> it's likely more invasive work.
> 
> The observation that led to the patch above was a much simpler one:
> since we now have the caller-locked __list_lru_del() anyway, the only
> difference between that and the list_lru_isolate* is the
> accounting. The patch is about one thing: removing the isolate helpers
> entirely. There is then simply one tool for caller-locked list
> unlinking, __list_lru_del(). It removes an avoidable distinction,
> shrinks the API surface, makes it harder to screw up.

I don't think replacing EXPORT_SYMBOL_GPL(list_lru_isolate) with
EXPORT_SYMBOL_GPL(__list_lru_del) makes much difference by itself.
The only reason I kept list_lru_isolate() in the current version
is to avoid passing an additional parameter (nid) to the isolate
callbacks.

I don't have a strong preference for the current approach. If we
can reach a consensus, I'd be happy to send v3 with the proposed
changes.

> The zswap weirdness made me look at list_lru. Consider it a stress
> test from idiotic code :-) But I would propose the list_lru
> simplification for its own sake, regardless of what we do with zswap.

-- 
Thanks
Kaitao Cheng