Re: [PATCH] mm/huge_memory: Fix iterator variable usage after swap()

["David Hildenbrand (Red Hat)" <david@kernel.org>]

On 1/21/26 10:25, Lance Yang wrote:
> 
> 
> On 2026/1/21 16:13, zenghongling wrote:
>> The iterator variable 'folio' is swapped with 'prev' in the else
>> branch. Using 'folio' after swap() checks the potentially NULL
>> 'prev' value, not the original iterator value.
>>
>> Fix by moving folio_put() call before the swap operation in the
>> path where swap() occurs.
>>
>> Found by:
>> ./huge_memory.c:4225:6-11: ERROR: iterator variable bound on line 4178 cannot be NULL
> 

Which tool did find that? A compiler?

> Good catch!
> 
> But which tree is your patch based on?
> 
> Seems like that was already fixed in commit 776bde7caf80[1]. The
> whole thing deferred_split_scan() was refactored using folio_batch,
> so the buggy code with swap(folio, prev) is gone ...
> 
> Ccing Muchun and Qi who fixed that.
> 
> [1]
> https://lore.kernel.org/all/59cb6b6fb5ffcff9d23b81890b252960139ad8e7.1762762324.git.zhengqi.arch@bytedance.com/

Right, in

commit 776bde7caf80f6af72b087cafe7d9f607b14716d
Author: Muchun Song <muchun.song@linux.dev>
Date:   Mon Nov 10 16:17:57 2025 +0800

     mm: thp: use folio_batch to handle THP splitting in deferred_split_scan()


Which raises the question whether we would want to backport that patch to stable kernels
if there was indeed a problem?


But: I don't immediately see the problem.

If pref is NULL (and folio obviously !+NULL), we'll end up with
	* pref != NULL
	* folio == NULL

	The "if (folio)" check will do nothing, because we defer the freeing to the

		if (prev)
			folio_put(prev);

	later

If pref is != NULL (and folio obviously !+NULL), we'll end up with
	* pref = NULL
	* folio = NULL

	The if (folio) and if (prev) handling will care of it all.


So ... this pretty much looks like working as expected?

-- 
Cheers

David