[PATCH 0/2] mm/filemap: reduce unnecessary xarray lookups

[PATCH 0/2] mm/filemap: reduce unnecessary xarray lookups

[Chi Zhiling]

From: Chi Zhiling <chizhiling@kylinos.cn>

This series optimizes xarray lookups in filemap by avoiding redundant
iterations after obtaining the last needed folio. The boundary check is
moved to before advancing the xarray iterator, eliminating unnecessary
lookups and branches in the fast path. This reduces the overhead of
filemap_get_read_batch() from 2.91% to 2.53% in 4k read tests.

This series is split from a previous patch series:
https://lore.kernel.org/linux-mm/20260601055704.167436-1-chizhiling@163.com/

Chi Zhiling (2):
  mm/filemap: reduce unnecessary xarray lookups when read cached pages
  mm/filemap: reduce unnecessary xarray lookups in filemap_get_folios_contig()

 mm/filemap.c | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

-- 
2.43.0

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

[Chi Zhiling]

From: Chi Zhiling <chizhiling@kylinos.cn>

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@infradead.org>
Signed-off-by: Chi Zhiling <chizhiling@kylinos.cn>
---
 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

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@kylinos.cn>
> 
> 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@infradead.org>
> Signed-off-by: Chi Zhiling <chizhiling@kylinos.cn>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								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@suse.com>
SUSE Labs, CR

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

[Chi Zhiling]

From: Chi Zhiling <chizhiling@kylinos.cn>

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@kylinos.cn>
---
 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

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@kylinos.cn>
> 
> 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@kylinos.cn>

Nice. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								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@suse.com>
SUSE Labs, CR