Re: [PATCH v2] mm: mglru: fix stale batch updates after memcg reparenting

[Harry Yoo <harry@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2045 bytes --]



On 6/25/26 3:11 PM, Qi Zheng wrote:
> On 6/25/26 12:16 PM, Harry Yoo wrote:
>>
> [...]
> 
>>
>>> So lock_batch_lruvec() can be implemented like this:
>>>
>>> #ifdef CONFIG_MEMCG
>>> static struct lruvec *lock_batch_lruvec(struct lruvec *lruvec)
>>> {
>>>      struct pglist_data *pgdat = lruvec_pgdat(lruvec);
>>>      struct mem_cgroup *memcg = lruvec_memcg(lruvec);
>>>
>>>      rcu_read_lock();
>>>
>>>      /*
>>>       * The memcg can be NULL when the memory controller is disabled.
>>>       * Otherwise, the caller keeps the memcg owning @lruvec alive.
>>>       */
>>>      if (!memcg || !css_is_dying(&memcg->css))
>>>          goto lock;
>>>
>>>      do {
>>>          memcg = parent_mem_cgroup(memcg);
>>>      } while (memcg && css_is_dying(&memcg->css));
>>>      lruvec = mem_cgroup_lruvec(memcg, pgdat);
>>>
>>> lock:
>>>      spin_lock_irq(&lruvec->lru_lock);
>>>
>>>      return lruvec;
>>> }
>>> #else
>>> static struct lruvec *lock_batch_lruvec(struct lruvec *lruvec)
>>> {
>>>      lruvec_lock_irq(lruvec);
>>>
>>>      return lruvec;
>>> }
>>> #endif
>>>
>>> Does this make sense?
>>
>> Yes, looks good to me!
> 
> OK, this sync method makes more sense as it doesn't require adding a
> new lrugen->reparente. I'll go with this method and update v3.

Thanks!

Just one thing to clarify...

So, when we check something that's updated _before_ grace period
(CSS_DYING), RCU is sufficient.

But in folio_lruvec_lock*(), that is not the case because reparenting
is performed in the RCU work, under the lruvec lock. So the check needs
to be done under RCU and the lruvec lock.

This is quite subtle :D

> Hi Barry and Baolin, what do you think? Since the sync method has been
> changed, I will temporarily drop your previous Reviewed-by tags in v3. ;)

And hopefully Peiyang would kindly double check v3 still not reproduced
on the machine :)

-- 
Cheers,
Harry / Hyeonggon


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]