Re: [Patch v3 2/4] mm/huge_memory: update folio stat after successful split

[Wei Yang <richard.weiyang@gmail.com>]

On Fri, Oct 24, 2025 at 03:32:42PM +0100, Lorenzo Stoakes wrote:
[...]
>> @@ -3636,49 +3631,32 @@ static int __split_unmapped_folio(struct folio *folio, int new_order,
>>  			else {
>>  				xas_set_order(xas, folio->index, split_order);
>>  				xas_try_split(xas, folio, old_order);
>> -				if (xas_error(xas)) {
>> -					ret = xas_error(xas);
>> -					stop_split = true;
>> -				}
>> +				if (xas_error(xas))
>> +					return xas_error(xas);
>>  			}
>>  		}
>>
>> -		if (!stop_split) {
>> -			folio_split_memcg_refs(folio, old_order, split_order);
>> -			split_page_owner(&folio->page, old_order, split_order);
>> -			pgalloc_tag_split(folio, old_order, split_order);
>> -
>> -			__split_folio_to_order(folio, old_order, split_order);
>> -		}
>> +		folio_split_memcg_refs(folio, old_order, split_order);
>> +		split_page_owner(&folio->page, old_order, split_order);
>> +		pgalloc_tag_split(folio, old_order, split_order);
>> +		__split_folio_to_order(folio, old_order, split_order);
>>
>> +		if (is_anon)
>> +			mod_mthp_stat(old_order, MTHP_STAT_NR_ANON, -1);
>>  		/*
>>  		 * Iterate through after-split folios and update folio stats.
>> -		 * But in buddy allocator like split, the folio
>> -		 * containing the specified page is skipped until its order
>> -		 * is new_order, since the folio will be worked on in next
>> -		 * iteration.
>>  		 */
>>  		for (new_folio = folio; new_folio != end_folio; new_folio = next) {
>>  			next = folio_next(new_folio);
>> -			/*
>> -			 * for buddy allocator like split, new_folio containing
>> -			 * @split_at page could be split again, thus do not
>> -			 * change stats yet. Wait until new_folio's order is
>> -			 * @new_order or stop_split is set to true by the above
>> -			 * xas_split() failure.
>> -			 */
>> -			if (new_folio == page_folio(split_at)) {
>> +			if (new_folio == page_folio(split_at))
>>  				folio = new_folio;
>> -				if (split_order != new_order && !stop_split)
>
>OK I guess we don't need this as in !uniform_split case we use
>xas_set_order() to set the order, then try to split, and if an error arose
>we already would have handled, so split_order == new_order is guaranteed at
>this point, right?
>

Sorry for the late reply, I was on travel.

In non-uniform splitting, @split_order represents an intermediate step, with
@new_order as the final target size. 

The check for split_order != new_order is necessary for the final split
iteration.

If this check is absent, the logic that updates folio statistics for the newly
split folios can be incorrectly bypassed when the !stop_split condition is
met.

Not sure I get your point correctly.

>> -					continue;
>> -			}
>>  			if (is_anon)
>>  				mod_mthp_stat(folio_order(new_folio),
>>  					      MTHP_STAT_NR_ANON, 1);
>>  		}
>>  	}
>>
>> -	return ret;
>> +	return 0;
>>  }
>>
>>  bool non_uniform_split_supported(struct folio *folio, unsigned int new_order,
>> --
>> 2.34.1
>>
>>

-- 
Wei Yang
Help you, Help me