Re: [PATCH 1/6] mm/page_owner: extract page_owner_next_eligible() from read loop

["zhen.ni" <zhen.ni@easystack.cn>]

在 2026/10/9 19:35, sashiko-bot@kernel.org 写道:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] mm: soft lockup in page_owner_next_eligible() due to missing cond_resched()
> --
> 
> commit 303a10c305ed333982228df86334f12bd6d8a3c2
> Author: Zhen Ni <zhen.ni@easystack.cn>
> 
> mm/page_owner: extract page_owner_next_eligible() from read loop
> 
> This commit extracts the page scanning logic from read_page_owner() into a
> dedicated helper, page_owner_next_eligible(). It advances a cursor to the
> next eligible page and takes a snapshot of the page_owner record inside the
> page_ext RCU window.
> 
>> diff --git a/mm/page_owner.c b/mm/page_owner.c
>> index fbbda7ba914ba..bafe474f33602 100644
>> --- a/mm/page_owner.c
>> +++ b/mm/page_owner.c
> 
> [ ... ]
> 
>> @@ -798,37 +795,67 @@ read_page_owner(struct file *file, char __user *buf, size_t count, loff_t *ppos)
>>   			goto ext_put_continue;
>>   
>>   		/*
>> -		 * Access to page_ext->handle isn't synchronous so we should
>> -		 * be careful to access it.
>> +		 * Take a snapshot of the page_owner record inside the RCU
>> +		 * window, then validate it, so that print_page_owner()
>> +		 * reads the checked handle from the same snapshot.
>>   		 */
>> -		handle = READ_ONCE(page_owner->handle);
>> -		if (!handle)
>> -			goto ext_put_continue;
>> +		scan->pfn = pfn;
>> +		scan->page = page;
>> +		scan->po_snap = *page_owner;
>> +		page_ext_put(page_ext);
>>   
>> +		if (!scan->po_snap.handle)
>> +			continue;
> 
> [Severity: High]
> Could this cause a soft lockup? By using a continue statement here in
> page_owner_next_eligible(), we jump directly to the next loop iteration
> (advancing pfn) without executing cond_resched() at the end of the loop
> block.
> 
> If a large contiguous sequence of allocated pages lacks stack handles
> (e.g., if stack depot is exhausted or during early non-tracked allocations),
> the kernel could iterate over millions of pages without yielding.
> 
> The original implementation used goto ext_put_continue; which correctly
> yielded. Does this need to be updated to explicitly yield or use the
> goto instead?
> 

Many contiguous allocated pages having a zero handleis an extreme
case, so I didn't add a scheduling point there.

It is harmless and keeps the reject path uniform, so I'll switch it to
`goto ext_put_continue` in the next revision.

>> +
>> +		return true;
>> +
>> +ext_put_continue:
>> +		page_ext_put(page_ext);
>> +		cond_resched();
>> +	}
> 

Thanks,
Zhen