Re: [PATCH 1/2] mm/filemap: reduce unnecessary xarray lookups when read cached pages

[Jan Kara]

On Sat 20-06-26 14:24:45, Chi Zhiling wrote:
> From: Chi Zhiling <chizhiling@xxxxxxxxxx>
> 
> When reading small amounts of data from the page cache, only a single
> folio is typically returned from filemap_read_get_batch(). In this case,
> calling xas_advance() or xas_next() after adding the folio to the batch
> is unnecessary and only introduces extra branches.
> 
> The same issue exists for large reads, where one additional xarray walk
> is always performed before termination.
> 
> Quit the loop once we get the last folio in the range, so the final
> redundant xarray advancement can be avoided.
> 
> The xas_next() does not update xa_index when xas->xa_node is set to
> XAS_RESTART, so the put and retry path would not update xa_index,
> hence the warning should therefore never trigger.
> 
> During the 4k reads test, the overhead of this function dropped from
> 2.91% to 2.53%.
> 
> Suggested-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
> Signed-off-by: Chi Zhiling <chizhiling@xxxxxxxxxx>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

> ---
>  mm/filemap.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 09cf51fd43b2..199c835c68bf 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -2467,11 +2467,14 @@ static void filemap_get_read_batch(struct address_space *mapping,
>  	XA_STATE(xas, &mapping->i_pages, index);
>  	struct folio *folio;
>  
> +	if (index > max)
> +		return;
> +
>  	rcu_read_lock();
>  	for (folio = xas_load(&xas); folio; folio = xas_next(&xas)) {
>  		if (xas_retry(&xas, folio))
>  			continue;
> -		if (xas.xa_index > max || xa_is_value(folio))
> +		if (xa_is_value(folio))
>  			break;
>  		if (xa_is_sibling(folio))
>  			break;
> @@ -2488,6 +2491,8 @@ static void filemap_get_read_batch(struct address_space *mapping,
>  		if (folio_test_readahead(folio))
>  			break;
>  		xas_advance(&xas, folio_next_index(folio) - 1);
> +		if (xas.xa_index >= max)
> +			break;
>  		continue;
>  put_folio:
>  		folio_put(folio);
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR