Re: [PATCH RFC 05/13] mm/huge_memory: consolidate irq and locking for folio split

[Zi Yan]

On Fri Aug 7, 2026 at 5:17 PM EDT, 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 paths follow the same convention and __folio_split()
> can drop its local irq handling and fail label, preparing for further
> cleanup.
>
> Signed-off-by: Kairui Song <kasong@xxxxxxxxxxx>
> ---
>  mm/huge_memory.c | 52 ++++++++++++++++++++++++----------------------------
>  1 file changed, 24 insertions(+), 28 deletions(-)
>

<snip>

anon part is not changed.

> @@ -4035,8 +4039,21 @@ static int __folio_freeze_split_unmapped_file(struct folio *folio, unsigned int
>  	struct lruvec *lruvec;
>  	int ret;
>  
> -	if (!folio_ref_freeze(folio, folio_cache_ref_count(folio) + 1))
> -		return -EAGAIN;
> +	xas_lock_irq(xas);

local_irq_disable() + xas_lock() is replaced by xas_lock_irq(). Are they
equivalent? Codex told me that the latter no longer prevents preemption
on RT kernel and can lengthen folio split process if it is preempted. It
is worth spell out in the commit message after you verify it.

> +
> +	/*
> +	 * Check if the folio is present in page cache.
> +	 * We assume all tail are present too, if folio is there.
> +	 */

xas_reset() is gone here. It seems to be a no-op, since xas is not
walked yet. But it is better to mention it in the commit message.

> +	if (xas_load(xas) != folio) {
> +		ret = -EAGAIN;
> +		goto fail;
> +	}
> +
> +	if (!folio_ref_freeze(folio, folio_cache_ref_count(folio) + 1)) {
> +		ret = -EAGAIN;
> +		goto fail;
> +	}
>  
>  	if (folio_test_pmd_mappable(folio) &&
>  	    new_order < HPAGE_PMD_ORDER) {
> @@ -4107,6 +4124,8 @@ static int __folio_freeze_split_unmapped_file(struct folio *folio, unsigned int
>  	if (do_lru)
>  		lruvec_unlock(lruvec);
>  
> +fail:
> +	xas_unlock_irq(xas);
>  	return ret;
>  }
>  
> @@ -4246,19 +4265,7 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
>  
>  	unmap_folio(folio);
>  
> -	/* block interrupt reentry in xa_lock and spinlock */
> -	local_irq_disable();
> -	if (mapping) {
> -		/*
> -		 * Check if the folio is present in page cache.
> -		 * We assume all tail are present too, if folio is there.
> -		 */
> -		xas_lock(&xas);
> -		xas_reset(&xas);
> -		if (xas_load(&xas) != folio) {
> -			ret = -EAGAIN;
> -			goto fail;
> -		}
> +	if (!is_anon) {
>  		ret = __folio_freeze_split_unmapped_file(folio, new_order, split_at, &xas, mapping,
>  							 true, list, split_type, end,
>  							 &nr_shmem_dropped);
> @@ -4267,12 +4274,6 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
>  							 list, split_type);
>  	}
>  
> -fail:
> -	if (mapping)
> -		xas_unlock(&xas);
> -
> -	local_irq_enable();
> -
>  	if (nr_shmem_dropped)
>  		shmem_uncharge(mapping->host, nr_shmem_dropped);
>  
> @@ -4355,8 +4356,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);
> @@ -4365,11 +4364,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_unmapped_anon(folio, new_order, &folio->page,
> -						 false, NULL, SPLIT_TYPE_UNIFORM);
> -	local_irq_enable();
> -	return ret;
> +	return __folio_freeze_split_unmapped_anon(folio, new_order, &folio->page,
> +						  false, NULL, SPLIT_TYPE_UNIFORM);
>  }
>  
>  /*

It is nice to see caller no longer needs to care about
local_irq_disable/enable().



-- 
Best Regards,
Yan, Zi