Re: [PATCH 2/2] mm/filemap: reduce unnecessary xarray lookups in filemap_get_folios_contig()

[Jan Kara]

On Sat 20-06-26 14:24:46, Chi Zhiling wrote:
> From: Chi Zhiling <chizhiling@xxxxxxxxxx>
> 
> Apply the same optimization used in filemap_get_read_batch() by moving
> the boundary check from the loop condition to before xas_next(),
> avoiding an unnecessary xarray lookup and reducing branches in the fast
> path.
> 
> Signed-off-by: Chi Zhiling <chizhiling@xxxxxxxxxx>

Nice. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

> ---
>  mm/filemap.c | 29 ++++++++++++++---------------
>  1 file changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 199c835c68bf..4ee591bc7e71 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -2270,10 +2270,11 @@ unsigned filemap_get_folios_contig(struct address_space *mapping,
>  	unsigned long nr;
>  	struct folio *folio;
>  
> -	rcu_read_lock();
> +	if (*start > end)
> +		return 0;
>  
> -	for (folio = xas_load(&xas); folio && xas.xa_index <= end;
> -			folio = xas_next(&xas)) {
> +	rcu_read_lock();
> +	for (folio = xas_load(&xas); folio; folio = xas_next(&xas)) {
>  		if (xas_retry(&xas, folio))
>  			continue;
>  		/*
> @@ -2281,11 +2282,11 @@ unsigned filemap_get_folios_contig(struct address_space *mapping,
>  		 * No current caller is looking for DAX entries.
>  		 */
>  		if (xa_is_value(folio))
> -			goto update_start;
> +			break;
>  
>  		/* If we landed in the middle of a THP, continue at its end. */
>  		if (xa_is_sibling(folio))
> -			goto update_start;
> +			break;
>  
>  		if (!folio_try_get(folio))
>  			goto retry;
> @@ -2293,29 +2294,27 @@ unsigned filemap_get_folios_contig(struct address_space *mapping,
>  		if (unlikely(folio != xas_reload(&xas)))
>  			goto put_folio;
>  
> -		if (!folio_batch_add(fbatch, folio)) {
> -			*start = folio_next_index(folio);
> -			goto out;
> -		}
> +		if (!folio_batch_add(fbatch, folio))
> +			break;
> +
>  		xas_advance(&xas, folio_next_index(folio) - 1);
> +		if (xas.xa_index >= end)
> +			break;
>  		continue;
> +
>  put_folio:
>  		folio_put(folio);
> -
>  retry:
>  		xas_reset(&xas);
>  	}
> +	rcu_read_unlock();
>  
> -update_start:
>  	nr = folio_batch_count(fbatch);
> -
>  	if (nr) {
>  		folio = fbatch->folios[nr - 1];
>  		*start = folio_next_index(folio);
>  	}
> -out:
> -	rcu_read_unlock();
> -	return folio_batch_count(fbatch);
> +	return nr;
>  }
>  EXPORT_SYMBOL(filemap_get_folios_contig);
>  
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR