Re: [PATCH 6/7] mm/filemap: allocate folios with mapping blocksize

[Pankaj Raghav <p.raghav@samsung.com>]

[-- Attachment #1: Type: text/plain, Size: 2490 bytes --]

Hi Hannes,
On Wed, Jun 14, 2023 at 01:46:36PM +0200, Hannes Reinecke wrote:
> The mapping has an underlying blocksize (by virtue of
> mapping->host->i_blkbits), so if the mapping blocksize
> is larger than the pagesize we should allocate folios
> in the correct order.
> 
Network filesystems such as 9pfs set the blkbits to be maximum data it
wants to transfer leading to unnecessary memory pressure as we will try
to allocate higher order folios(Order 5 in my setup). Isn't it better
for each filesystem to request the minimum folio order it needs for its
page cache early on? Block devices can do the same for its block cache.

I have prototype along those lines and I will it soon. This is also
something willy indicated before in a mailing list conversation.

> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
> diff --git a/mm/readahead.c b/mm/readahead.c
> index 47afbca1d122..031935b78af7 100644
> --- a/mm/readahead.c
> +++ b/mm/readahead.c
> @@ -245,7 +245,7 @@ void page_cache_ra_unbounded(struct readahead_control *ractl,
>  			continue;
>  		}
>  
> -		folio = filemap_alloc_folio(gfp_mask, 0);
> +		folio = filemap_alloc_folio(gfp_mask, mapping_get_order(mapping));
>  		if (!folio)
>  			break;
>  		if (filemap_add_folio(mapping, folio, index + i,

Did you turn on CONFIG_DEBUG_VM while testing? I don't think we are
incrementing the counter in this function correctly as this function
assumes order 0. We might need something like this:

-               ractl->_nr_pages++;
+               ractl->_nr_pages += folio_nr_pages(folio);
+               i += folio_nr_pages(folio) - 1;
> @@ -806,7 +806,7 @@ void readahead_expand(struct readahead_control *ractl,
>  		if (folio && !xa_is_value(folio))
>  			return; /* Folio apparently present */
>  
> -		folio = filemap_alloc_folio(gfp_mask, 0);
> +		folio = filemap_alloc_folio(gfp_mask, mapping_get_order(mapping));
>  		if (!folio)
>  			return;
>  		if (filemap_add_folio(mapping, folio, index, gfp_mask) < 0) {
> @@ -833,7 +833,7 @@ void readahead_expand(struct readahead_control *ractl,
>  		if (folio && !xa_is_value(folio))
>  			return; /* Folio apparently present */
Same here:
-               ractl->_nr_pages++;
+               ractl->_nr_pages += folio_nr_pages(folio);

>  
> -		folio = filemap_alloc_folio(gfp_mask, 0);
> +		folio = filemap_alloc_folio(gfp_mask, mapping_get_order(mapping));
>  		if (!folio)
>  			return;
>  		if (filemap_add_folio(mapping, folio, index, gfp_mask) < 0) {
> -- 
> 2.35.3
> 

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]