Re: [PATCH v5 4/4] mm: thp: reparent the split queue during memcg offline

[Qi Zheng <qi.zheng@linux.dev>]

On 10/21/25 5:29 PM, Harry Yoo wrote:
> On Tue, Oct 21, 2025 at 02:21:55PM +0800, Qi Zheng wrote:
>>
>>
>> On 10/21/25 2:09 PM, Harry Yoo wrote:
>>> On Wed, Oct 15, 2025 at 02:35:33PM +0800, Qi Zheng wrote:
>>>> From: Qi Zheng <zhengqi.arch@bytedance.com>
>>>>
>>>> Similar to list_lru, the split queue is relatively independent and does
>>>> not need to be reparented along with objcg and LRU folios (holding
>>>> objcg lock and lru lock). So let's apply the similar mechanism as list_lru
>>>> to reparent the split queue separately when memcg is offine.
>>>>
>>>> This is also a preparation for reparenting LRU folios.
>>>>
>>>> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
>>>> Acked-by: Zi Yan <ziy@nvidia.com>
>>>> Reviewed-by: Muchun Song <muchun.song@linux.dev>
>>>> Acked-by: David Hildenbrand <david@redhat.com>
>>>> Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
>>>> ---
>>>
>>> Looks good to me,
>>> Reviewed-by: Harry Yoo <harry.yoo@oracle.com>
>>
>> Thanks.
>>
>>>
>>> with a question:
>>>
>>>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>>>> index e850bc10da3e2..9323039418201 100644
>>>> --- a/mm/huge_memory.c
>>>> +++ b/mm/huge_memory.c
>>>> @@ -1117,8 +1117,19 @@ static struct deferred_split *split_queue_lock(int nid, struct mem_cgroup *memcg
>>>>    {
>>>>    	struct deferred_split *queue;
>>>> +retry:
>>>>    	queue = memcg_split_queue(nid, memcg);
>>>>    	spin_lock(&queue->split_queue_lock);
>>>> +	/*
>>>> +	 * There is a period between setting memcg to dying and reparenting
>>>> +	 * deferred split queue, and during this period the THPs in the deferred
>>>> +	 * split queue will be hidden from the shrinker side.
>>>> +	 */
>>>
>>> You mean it will be hidden if the shrinker bit is not set for the node
>>> in the parent memcg, right?
>>
>> Look at the following situation:
>>
>> CPU 0                   CPU 1
>> -----                   -----
>>
>> set CSS_DYING
>>                          deferred_split_scan
>>                              /*
>>                               * See CSS_DYING, and return the parent
>>                               * memcg's ds_queue. But the pages on the
>>                               * child memcg's ds_queue has not yet been
>>                               * reparented to the parent memcg, that is,
>>                               * it is hidden.
>>                               */
>>                          --> ds_queue = split_queue_lock_irqsave()
>>
>> reparent_deferred_split_queue
> 
> Ah, I see what you meant. Thanks.
> 
> So we may end up shrinking the parent memcg twice if it's
> hidden, but I guess it's fine as it'll be rare?

Yeah.

> 
>> Thanks,
>> Qi
>