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

[Peiyang He]

On 2026/6/25 15:37, Qi Zheng wrote:
> 
> 
> On 6/25/26 2:32 PM, Harry Yoo wrote:
>>
>>
>> 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
> 
> Indeed.
> 
> And in theory, the l->nr_items check in lock_list_lru_of_memcg() could
> also be replaced by the CSS_DYING check.
> 
>>
>>> 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 :)
> 
> Yeah!
No problem! I can help test v3.> 
>>
> 
>