Re: [PATCH v8 01/10] fs: Allow fine-grained control of folio sizes

Pankaj Raghav (Samsung)
2024-07-05T12:46:08Z

On Fri, Jul 05, 2024 at 10:03:31AM +0100, Ryan Roberts wrote:
> On 05/07/2024 05:32, Dave Chinner wrote:
> > On Fri, Jul 05, 2024 at 12:56:28AM +0100, Matthew Wilcox wrote:
> >> On Fri, Jul 05, 2024 at 08:06:51AM +1000, Dave Chinner wrote:
> >>>>> It seems strange to silently clamp these? Presumably for the bs>ps usecase,
> >>>>> whatever values are passed in are a hard requirement? So wouldn't want them to
> >>>>> be silently reduced. (Especially given the recent change to reduce the size of
> >>>>> MAX_PAGECACHE_ORDER to less then PMD size in some cases).
> >>>>
> >>>> Hm, yes. We should probably make this return an errno. Including
> >>>> returning an errno for !IS_ENABLED() and min > 0.
> >>>
> >>> What are callers supposed to do with an error? In the case of
> >>> setting up a newly allocated inode in XFS, the error would be
> >>> returned in the middle of a transaction and so this failure would
> >>> result in a filesystem shutdown.
> >>
> >> I suggest you handle it better than this. If the device is asking for a
> >> blocksize > PMD_SIZE, you should fail to mount it.
>
> A detail, but MAX_PAGECACHE_ORDER may be smaller than PMD_SIZE even on systems
> with CONFIG_TRANSPARENT_HUGEPAGE as of a fix that is currently in mm-unstable:

>
> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> #define PREFERRED_MAX_PAGECACHE_ORDER HPAGE_PMD_ORDER
> #else
> #define PREFERRED_MAX_PAGECACHE_ORDER 8
> #endif
>
> /*
> * xas_split_alloc() does not support arbitrary orders. This implies no
> * 512MB THP on ARM64 with 64KB base page size.
> */
> #define MAX_XAS_ORDER (XA_CHUNK_SHIFT * 2 - 1)
> #define MAX_PAGECACHE_ORDER min(MAX_XAS_ORDER,
> PREFERRED_MAX_PAGECACHE_ORDER)
>
> But that also implies that the page cache can handle up to order-8 without
> CONFIG_TRANSPARENT_HUGEPAGE so sounds like there isn't a dependcy on
> CONFIG_TRANSPARENT_HUGEPAGE in this respect?
>

I remember seeing willy's comment about dependency on CONFIG_TRANSPARENT_HUGEPAGE
for large folios[0]:

Some parts of the VM still depend on THP to handle large folios
correctly. Until those are fixed, prevent creating large folios
if THP are disabled.

This was Jan of 2022. I don't know the status of it now but we enable
large folios for filesystem only when THP is enabled(as you can see in
the helpers mapping_set_folio_order_range()).

[0] https://lore.kernel.org/lkml/20220116121822.1727633-8-willy@infradead.org/