Re: [PATCH v4 06/17] mm/huge_memory: consolidate irq and locking for folio split

[David Hildenbrand (Arm)]

On 9/7/26 20:12, Kairui Song via B4 Relay wrote:
> From: Kairui Song <kasong@xxxxxxxxxxx>
> 
> Let each split helper handle its own locking instead of relying on
> the caller, so both helpers manage their own irq and locking state.
> This lets __folio_split() drop its local irq handling and fail label,
> preparing for further cleanup.
> 
> The file path now uses xas_lock_irq() instead of local_irq_disable()
> with xas_lock(). The two are equivalent on non-RT, and
> TRANSPARENT_HUGEPAGE cannot be enabled on RT anyway. This conversion
> also buys consistency: every other place in mm/ that freezes a folio
> while it is still reachable through the page cache already takes the
> lock this way. This was actually the last plain xas_lock() on
> mapping->i_pages left in mm. If we are going to support RT, spinning
> on frozen folio refs could be a problem, but it already exists in
> many places and should be fixed generically.
> 
> The anon helper keeps a single local_irq_disable() as before, because
> it has to cover several plain spinlocks at once.
> 
> The dropped xas_reset() was a no-op as the xa_state is not walked
> before the xas_load() under the lock.
> 
> Reviewed-by: Zi Yan <ziy@xxxxxxxxxx>
> Reviewed-by: Kiryl Shutsemau (Meta) <kas@xxxxxxxxxx>
> Reviewed-by: Yeoreum Yun <yeoreum.yun@xxxxxxx>
> Signed-off-by: Kairui Song <kasong@xxxxxxxxxxx>
> ---

[...]

>  	if (nr_shmem_dropped)
>  		shmem_uncharge(mapping->host, nr_shmem_dropped);
> @@ -4390,8 +4391,6 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
>   */
>  int folio_split_unmapped(struct folio *folio, unsigned int new_order)
>  {
> -	int ret = 0;
> -
>  	VM_WARN_ON_ONCE_FOLIO(folio_mapped(folio), folio);
>  	VM_WARN_ON_ONCE_FOLIO(!folio_test_locked(folio), folio);
>  	VM_WARN_ON_ONCE_FOLIO(!folio_test_large(folio), folio);
> @@ -4400,11 +4399,8 @@ int folio_split_unmapped(struct folio *folio, unsigned int new_order)
>  	if (folio_expected_ref_count(folio) != folio_ref_count(folio) - 1)
>  		return -EAGAIN;
>  
> -	local_irq_disable();
> -	ret = __folio_freeze_split_anon(folio, new_order, &folio->page,
> -					false, NULL, SPLIT_TYPE_UNIFORM);
> -	local_irq_enable();
> -	return ret;
> +	return __folio_freeze_split_anon(folio, new_order, &folio->page,
> +					 false, NULL, SPLIT_TYPE_UNIFORM);

Yeah, that bit looks especially nice.

Acked-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>

-- 
Cheers,

David