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

[Hannes Reinecke <hare@suse.de>]

On 6/20/23 00:57, Dave Chinner wrote:
> On Mon, Jun 19, 2023 at 10:42:38AM +0200, Hannes Reinecke wrote:
>> On 6/19/23 10:08, Pankaj Raghav wrote:
>>> 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.
> 
> Folio size is not a "filesystem wide" thing - it's a per-inode
> configuration. We can have inodes within a filesystem that have
> different "block" sizes. A prime example of this is XFS directories
> - they can have 64kB block sizes on 4kB block size filesystem.
> 
> Another example is extent size hints in XFS data files - they
> trigger aligned allocation-around similar to using large folios in
> the page cache for small writes. Effectively this gives data files a
> "block size" of the extent size hint regardless of the filesystem
> block size.
> 
> Hence in future we might want different sizes of folios for
> different types of inodes and so whatever we do we need to support
> per-inode folio size configuration for the inode mapping tree.
> 
Sure. Using some mapping tree configuration was what I had in mind, too.

>>> I have prototype along those lines and I will it soon. This is also
>>> something willy indicated before in a mailing list conversation.
>>>
>> Well; I _though_ that's why we had things like optimal I/O size and
>> maximal I/O size. But this seem to be relegated to request queue limits,
>> so I guess it's not available from 'struct block_device' or 'struct
>> gendisk'.
> 
> Yes, those are block device constructs to enable block device based
> filesystems to be laid out best for the given block device. They
> don't exist for non-block-based filesystems like network
> filesystems...
> 
>> So I've been thinking of adding a flag somewhere (possibly in
>> 'struct address_space') to indicate that blkbits is a hard limit
>> and not just an advisory thing.
> 
> This still relies on interpreting inode->i_blkbits repeatedly at
> runtime in some way, in mm code that really has no business looking
> at filesystem block sizes.
> 
> What is needed is a field into the mapping that defines the
> folio order that all folios allocated for the page cache must be
> aligned/sized to to allow them to be inserted into the mapping.
> 
> This means the minimum folio order and alignment is maintained
> entirely by the mapping (e.g. it allows truncate to do the right
> thing), and the filesystem/device side code does not need to do
> anything special (except support large folios) to ensure that the
> page cache always contains folios that are block sized and aligned.
> 
> We already have mapping_set_large_folios() that we use at
> inode/mapping instantiation time to enable large folios in the page
> cache for that mapping. What we need is a new
> mapping_set_large_folio_order() API to enable the filesystem/device
> to set the base folio order for the mapping tree at instantiation
> time, and for all the page cache instantiation code to align/size to
> the order stored in the mapping...
> 
Having a mapping_set_large_folio_order() (or maybe 
mapping_set_folio_order(), for as soon as order > 0 we will have large
folios ...) looks like a good idea.
I'll give it a go.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman