| Message ID | 20230710130253.3484695-8-willy@infradead.org (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
| Series | Create large folios in iomap buffered write path | expand |
On Mon, Jul 10, 2023 at 02:02:51PM +0100, Matthew Wilcox (Oracle) wrote: > Allow callers of __filemap_get_folio() to specify a preferred folio > order in the FGP flags. This is only honoured in the FGP_CREATE path; > if there is already a folio in the page cache that covers the index, > we will return it, no matter what its order is. No create-around is > attempted; we will only create folios which start at the specified index. > Unmodified callers will continue to allocate order 0 folios. > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > Reviewed-by: Christoph Hellwig <hch@lst.de> > --- > include/linux/pagemap.h | 34 ++++++++++++++++++++++++++++++ > mm/filemap.c | 46 +++++++++++++++++++++++++++++------------ > mm/readahead.c | 13 ------------ > 3 files changed, 67 insertions(+), 26 deletions(-) > > diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h > index 911201fc41fc..d87840acbfb2 100644 > --- a/include/linux/pagemap.h > +++ b/include/linux/pagemap.h > @@ -470,6 +470,19 @@ static inline void *detach_page_private(struct page *page) > return folio_detach_private(page_folio(page)); > } > > +/* > + * There are some parts of the kernel which assume that PMD entries > + * are exactly HPAGE_PMD_ORDER. Those should be fixed, but until then, > + * limit the maximum allocation order to PMD size. I'm not aware of any > + * assumptions about maximum order if THP are disabled, but 8 seems like > + * a good order (that's 1MB if you're using 4kB pages) > + */ > +#ifdef CONFIG_TRANSPARENT_HUGEPAGE > +#define MAX_PAGECACHE_ORDER HPAGE_PMD_ORDER > +#else > +#define MAX_PAGECACHE_ORDER 8 > +#endif > + > #ifdef CONFIG_NUMA > struct folio *filemap_alloc_folio(gfp_t gfp, unsigned int order); > #else > @@ -535,9 +548,30 @@ typedef unsigned int __bitwise fgf_t; > #define FGP_NOWAIT ((__force fgf_t)0x00000020) > #define FGP_FOR_MMAP ((__force fgf_t)0x00000040) > #define FGP_STABLE ((__force fgf_t)0x00000080) > +#define FGF_GET_ORDER(fgf) (((__force unsigned)fgf) >> 26) /* top 6 bits */ > > #define FGP_WRITEBEGIN (FGP_LOCK | FGP_WRITE | FGP_CREAT | FGP_STABLE) > > +/** > + * fgf_set_order - Encode a length in the fgf_t flags. > + * @size: The suggested size of the folio to create. > + * > + * The caller of __filemap_get_folio() can use this to suggest a preferred > + * size for the folio that is created. If there is already a folio at > + * the index, it will be returned, no matter what its size. If a folio > + * is freshly created, it may be of a different size than requested > + * due to alignment constraints, memory pressure, or the presence of > + * other folios at nearby indices. > + */ > +static inline fgf_t fgf_set_order(size_t size) > +{ > + unsigned int shift = ilog2(size); > + > + if (shift <= PAGE_SHIFT) > + return 0; > + return (__force fgf_t)((shift - PAGE_SHIFT) << 26); > +} > + > void *filemap_get_entry(struct address_space *mapping, pgoff_t index); > struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index, > fgf_t fgp_flags, gfp_t gfp); > diff --git a/mm/filemap.c b/mm/filemap.c > index 8a669fecfd1c..baafbf324c9f 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -1905,7 +1905,9 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index, > folio_wait_stable(folio); > no_page: > if (!folio && (fgp_flags & FGP_CREAT)) { > + unsigned order = FGF_GET_ORDER(fgp_flags); > int err; > + > if ((fgp_flags & FGP_WRITE) && mapping_can_writeback(mapping)) > gfp |= __GFP_WRITE; > if (fgp_flags & FGP_NOFS) > @@ -1914,26 +1916,44 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index, > gfp &= ~GFP_KERNEL; > gfp |= GFP_NOWAIT | __GFP_NOWARN; > } > - > - folio = filemap_alloc_folio(gfp, 0); > - if (!folio) > - return ERR_PTR(-ENOMEM); > - > if (WARN_ON_ONCE(!(fgp_flags & (FGP_LOCK | FGP_FOR_MMAP)))) > fgp_flags |= FGP_LOCK; > > - /* Init accessed so avoid atomic mark_page_accessed later */ > - if (fgp_flags & FGP_ACCESSED) > - __folio_set_referenced(folio); > + if (!mapping_large_folio_support(mapping)) > + order = 0; > + if (order > MAX_PAGECACHE_ORDER) > + order = MAX_PAGECACHE_ORDER; Curious how this ended up being the heuristic used to shoot for the MAX_PAGECACHE_ORDER sky first, and then go down 1/2 each time. I don't see it explained on the commit log but I'm sure there's has to be some reasonable rationale. From the cover letter, I could guess that it means the gains of always using the largest folio possible means an implied latency savings through other means, so the small latencies spent looking seem to no where compare to the saving in using. But I rather understand a bit more for the rationale. Are there situations where perhaps limitting this initial max preferred high order folio might be smaller than MAX_PAGECACHE_ORDER? How if not, how do we know? Luis
On Mon, Jul 10, 2023 at 04:58:55PM -0700, Luis Chamberlain wrote: > > @@ -1914,26 +1916,44 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index, > > gfp &= ~GFP_KERNEL; > > gfp |= GFP_NOWAIT | __GFP_NOWARN; > > } > > - > > - folio = filemap_alloc_folio(gfp, 0); > > - if (!folio) > > - return ERR_PTR(-ENOMEM); > > - > > if (WARN_ON_ONCE(!(fgp_flags & (FGP_LOCK | FGP_FOR_MMAP)))) > > fgp_flags |= FGP_LOCK; > > > > - /* Init accessed so avoid atomic mark_page_accessed later */ > > - if (fgp_flags & FGP_ACCESSED) > > - __folio_set_referenced(folio); > > + if (!mapping_large_folio_support(mapping)) > > + order = 0; > > + if (order > MAX_PAGECACHE_ORDER) > > + order = MAX_PAGECACHE_ORDER; > > Curious how this ended up being the heuristic used to shoot for the > MAX_PAGECACHE_ORDER sky first, and then go down 1/2 each time. I don't > see it explained on the commit log but I'm sure there's has to be > some reasonable rationale. From the cover letter, I could guess that > it means the gains of always using the largest folio possible means > an implied latency savings through other means, so the small latencies > spent looking seem to no where compare to the saving in using. But > I rather understand a bit more for the rationale. You've completely misunderstood this patch. The caller hints at the folio size it wants, and this code you're highlighting limits it to be less than MAX_PAGECACHE_ORDER.
On Mon, Jul 10, 2023 at 04:58:55PM -0700, Luis Chamberlain wrote: > Curious how this ended up being the heuristic used to shoot for the > MAX_PAGECACHE_ORDER sky first, and then go down 1/2 each time. I don't > see it explained on the commit log but I'm sure there's has to be > some reasonable rationale. From the cover letter, I could guess that > it means the gains of always using the largest folio possible means > an implied latency savings through other means, so the small latencies > spent looking seem to no where compare to the saving in using. But > I rather understand a bit more for the rationale. > > Are there situations where perhaps limitting this initial max preferred > high order folio might be smaller than MAX_PAGECACHE_ORDER? How if not, > how do we know? It's the exact same situation as when filesystems all switched from blocks to extents 10-20 years ago. We _always_ want our allocations to be as big as possible - it reduces metadata overhead, preserves locality, reduces fragmentation - IOW, this is clearly the right thing to do.
On Tue, Jul 11, 2023 at 01:07:37AM +0100, Matthew Wilcox wrote: > The caller hints at the > folio size it wants, and this code you're highlighting limits it to be > less than MAX_PAGECACHE_ORDER. Yes sorry, if the write size is large we still max out at MAX_PAGECACHE_ORDER naturally. I don't doubt the rationale that that is a good idea. What I'm curious about is why are we OK to jump straight to MAX_PAGECACHE_ORDER from the start and if there are situations where perhaps a a not-so-aggressive high order may be desriable. How do we know? Luis
On Mon, Jul 10, 2023 at 05:21:04PM -0700, Luis Chamberlain wrote: > On Tue, Jul 11, 2023 at 01:07:37AM +0100, Matthew Wilcox wrote: > > The caller hints at the > > folio size it wants, and this code you're highlighting limits it to be > > less than MAX_PAGECACHE_ORDER. > > Yes sorry, if the write size is large we still max out at MAX_PAGECACHE_ORDER > naturally. I don't doubt the rationale that that is a good idea. > > What I'm curious about is why are we OK to jump straight to MAX_PAGECACHE_ORDER > from the start and if there are situations where perhaps a a > not-so-aggressive high order may be desriable. How do we know? The page cache trusts the filesystem to make a good guess. Dave had some ideas about how XFS might make different guesses from the one in this patchset, and I encourage people to experiment with different algorithms. Intuitively "size of the write" is probably not a bad guess. If userspace is writing 400kB in a single write, it proabbly makes sense to writeback & age and eventually reclaim that entire 400kB at the same time. If we guess wrong, the downside probably isn't too bad either. But we need data! And merging this patch set & gathering real world experience with it is a good start.
On Mon, Jul 10, 2023 at 05:21:04PM -0700, Luis Chamberlain wrote: > On Tue, Jul 11, 2023 at 01:07:37AM +0100, Matthew Wilcox wrote: > > The caller hints at the > > folio size it wants, and this code you're highlighting limits it to be > > less than MAX_PAGECACHE_ORDER. > > Yes sorry, if the write size is large we still max out at MAX_PAGECACHE_ORDER > naturally. I don't doubt the rationale that that is a good idea. > > What I'm curious about is why are we OK to jump straight to MAX_PAGECACHE_ORDER > from the start and if there are situations where perhaps a a > not-so-aggressive high order may be desriable. How do we know? Decades of experience optimising IO and allocation on extent based filesystems tell us this is the right thing to do. i.e. we should always attempt to allocate the largest contiguous range possible for the current operation being performed. This almost always results in the minimum amount of metadata and overhead to index and manage the data being stored. There are relatively few situations where this is not the right thing to do, but that is where per-inode behavioural control options like extent size hints come into play.... Cheers, Dave.
On Mon, Jul 10, 2023 at 02:02:51PM +0100, Matthew Wilcox (Oracle) wrote: > Allow callers of __filemap_get_folio() to specify a preferred folio > order in the FGP flags. This is only honoured in the FGP_CREATE path; > if there is already a folio in the page cache that covers the index, > we will return it, no matter what its order is. No create-around is > attempted; we will only create folios which start at the specified index. > Unmodified callers will continue to allocate order 0 folios. > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > Reviewed-by: Christoph Hellwig <hch@lst.de> > --- > include/linux/pagemap.h | 34 ++++++++++++++++++++++++++++++ > mm/filemap.c | 46 +++++++++++++++++++++++++++++------------ > mm/readahead.c | 13 ------------ > 3 files changed, 67 insertions(+), 26 deletions(-) > > diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h > index 911201fc41fc..d87840acbfb2 100644 > --- a/include/linux/pagemap.h > +++ b/include/linux/pagemap.h > @@ -470,6 +470,19 @@ static inline void *detach_page_private(struct page *page) > return folio_detach_private(page_folio(page)); > } > > +/* > + * There are some parts of the kernel which assume that PMD entries > + * are exactly HPAGE_PMD_ORDER. Those should be fixed, but until then, > + * limit the maximum allocation order to PMD size. I'm not aware of any > + * assumptions about maximum order if THP are disabled, but 8 seems like > + * a good order (that's 1MB if you're using 4kB pages) > + */ > +#ifdef CONFIG_TRANSPARENT_HUGEPAGE > +#define MAX_PAGECACHE_ORDER HPAGE_PMD_ORDER > +#else > +#define MAX_PAGECACHE_ORDER 8 > +#endif > + > #ifdef CONFIG_NUMA > struct folio *filemap_alloc_folio(gfp_t gfp, unsigned int order); > #else > @@ -535,9 +548,30 @@ typedef unsigned int __bitwise fgf_t; > #define FGP_NOWAIT ((__force fgf_t)0x00000020) > #define FGP_FOR_MMAP ((__force fgf_t)0x00000040) > #define FGP_STABLE ((__force fgf_t)0x00000080) > +#define FGF_GET_ORDER(fgf) (((__force unsigned)fgf) >> 26) /* top 6 bits */ /me is very excited for xfs to be able to ask for bigger folios for big allocations. Playing with 512M folios on arm64 is very exciting, especially with Ritesh's patches to reduce write amplification. (Surprisingly, so far the only fstest that's broken is xfs/559, and only because it assumed that pagecache writes only create basepages.) Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > > #define FGP_WRITEBEGIN (FGP_LOCK | FGP_WRITE | FGP_CREAT | FGP_STABLE) > > +/** > + * fgf_set_order - Encode a length in the fgf_t flags. > + * @size: The suggested size of the folio to create. > + * > + * The caller of __filemap_get_folio() can use this to suggest a preferred > + * size for the folio that is created. If there is already a folio at > + * the index, it will be returned, no matter what its size. If a folio > + * is freshly created, it may be of a different size than requested > + * due to alignment constraints, memory pressure, or the presence of > + * other folios at nearby indices. > + */ > +static inline fgf_t fgf_set_order(size_t size) > +{ > + unsigned int shift = ilog2(size); > + > + if (shift <= PAGE_SHIFT) > + return 0; > + return (__force fgf_t)((shift - PAGE_SHIFT) << 26); > +} > + > void *filemap_get_entry(struct address_space *mapping, pgoff_t index); > struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index, > fgf_t fgp_flags, gfp_t gfp); > diff --git a/mm/filemap.c b/mm/filemap.c > index 8a669fecfd1c..baafbf324c9f 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -1905,7 +1905,9 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index, > folio_wait_stable(folio); > no_page: > if (!folio && (fgp_flags & FGP_CREAT)) { > + unsigned order = FGF_GET_ORDER(fgp_flags); > int err; > + > if ((fgp_flags & FGP_WRITE) && mapping_can_writeback(mapping)) > gfp |= __GFP_WRITE; > if (fgp_flags & FGP_NOFS) > @@ -1914,26 +1916,44 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index, > gfp &= ~GFP_KERNEL; > gfp |= GFP_NOWAIT | __GFP_NOWARN; > } > - > - folio = filemap_alloc_folio(gfp, 0); > - if (!folio) > - return ERR_PTR(-ENOMEM); > - > if (WARN_ON_ONCE(!(fgp_flags & (FGP_LOCK | FGP_FOR_MMAP)))) > fgp_flags |= FGP_LOCK; > > - /* Init accessed so avoid atomic mark_page_accessed later */ > - if (fgp_flags & FGP_ACCESSED) > - __folio_set_referenced(folio); > + if (!mapping_large_folio_support(mapping)) > + order = 0; > + if (order > MAX_PAGECACHE_ORDER) > + order = MAX_PAGECACHE_ORDER; > + /* If we're not aligned, allocate a smaller folio */ > + if (index & ((1UL << order) - 1)) > + order = __ffs(index); > > - err = filemap_add_folio(mapping, folio, index, gfp); > - if (unlikely(err)) { > + do { > + gfp_t alloc_gfp = gfp; > + > + err = -ENOMEM; > + if (order == 1) > + order = 0; > + if (order > 0) > + alloc_gfp |= __GFP_NORETRY | __GFP_NOWARN; > + folio = filemap_alloc_folio(alloc_gfp, order); > + if (!folio) > + continue; > + > + /* Init accessed so avoid atomic mark_page_accessed later */ > + if (fgp_flags & FGP_ACCESSED) > + __folio_set_referenced(folio); > + > + err = filemap_add_folio(mapping, folio, index, gfp); > + if (!err) > + break; > folio_put(folio); > folio = NULL; > - if (err == -EEXIST) > - goto repeat; > - } > + } while (order-- > 0); > > + if (err == -EEXIST) > + goto repeat; > + if (err) > + return ERR_PTR(err); > /* > * filemap_add_folio locks the page, and for mmap > * we expect an unlocked page. > diff --git a/mm/readahead.c b/mm/readahead.c > index a9c999aa19af..e815c114de21 100644 > --- a/mm/readahead.c > +++ b/mm/readahead.c > @@ -461,19 +461,6 @@ static int try_context_readahead(struct address_space *mapping, > return 1; > } > > -/* > - * There are some parts of the kernel which assume that PMD entries > - * are exactly HPAGE_PMD_ORDER. Those should be fixed, but until then, > - * limit the maximum allocation order to PMD size. I'm not aware of any > - * assumptions about maximum order if THP are disabled, but 8 seems like > - * a good order (that's 1MB if you're using 4kB pages) > - */ > -#ifdef CONFIG_TRANSPARENT_HUGEPAGE > -#define MAX_PAGECACHE_ORDER HPAGE_PMD_ORDER > -#else > -#define MAX_PAGECACHE_ORDER 8 > -#endif > - > static inline int ra_alloc_folio(struct readahead_control *ractl, pgoff_t index, > pgoff_t mark, unsigned int order, gfp_t gfp) > { > -- > 2.39.2 >
On Mon, Jul 10, 2023 at 02:02:51PM +0100, Matthew Wilcox (Oracle) wrote: > Allow callers of __filemap_get_folio() to specify a preferred folio > order in the FGP flags. This is only honoured in the FGP_CREATE path; > if there is already a folio in the page cache that covers the index, > we will return it, no matter what its order is. No create-around is > attempted; we will only create folios which start at the specified index. > Unmodified callers will continue to allocate order 0 folios. Why not just add an end_index parameter to filemap_get_folio()? > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > Reviewed-by: Christoph Hellwig <hch@lst.de> > --- > include/linux/pagemap.h | 34 ++++++++++++++++++++++++++++++ > mm/filemap.c | 46 +++++++++++++++++++++++++++++------------ > mm/readahead.c | 13 ------------ > 3 files changed, 67 insertions(+), 26 deletions(-) > > diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h > index 911201fc41fc..d87840acbfb2 100644 > --- a/include/linux/pagemap.h > +++ b/include/linux/pagemap.h > @@ -470,6 +470,19 @@ static inline void *detach_page_private(struct page *page) > return folio_detach_private(page_folio(page)); > } > > +/* > + * There are some parts of the kernel which assume that PMD entries > + * are exactly HPAGE_PMD_ORDER. Those should be fixed, but until then, > + * limit the maximum allocation order to PMD size. I'm not aware of any > + * assumptions about maximum order if THP are disabled, but 8 seems like > + * a good order (that's 1MB if you're using 4kB pages) > + */ > +#ifdef CONFIG_TRANSPARENT_HUGEPAGE > +#define MAX_PAGECACHE_ORDER HPAGE_PMD_ORDER > +#else > +#define MAX_PAGECACHE_ORDER 8 > +#endif > + > #ifdef CONFIG_NUMA > struct folio *filemap_alloc_folio(gfp_t gfp, unsigned int order); > #else > @@ -535,9 +548,30 @@ typedef unsigned int __bitwise fgf_t; > #define FGP_NOWAIT ((__force fgf_t)0x00000020) > #define FGP_FOR_MMAP ((__force fgf_t)0x00000040) > #define FGP_STABLE ((__force fgf_t)0x00000080) > +#define FGF_GET_ORDER(fgf) (((__force unsigned)fgf) >> 26) /* top 6 bits */ > > #define FGP_WRITEBEGIN (FGP_LOCK | FGP_WRITE | FGP_CREAT | FGP_STABLE) > > +/** > + * fgf_set_order - Encode a length in the fgf_t flags. > + * @size: The suggested size of the folio to create. > + * > + * The caller of __filemap_get_folio() can use this to suggest a preferred > + * size for the folio that is created. If there is already a folio at > + * the index, it will be returned, no matter what its size. If a folio > + * is freshly created, it may be of a different size than requested > + * due to alignment constraints, memory pressure, or the presence of > + * other folios at nearby indices. > + */ > +static inline fgf_t fgf_set_order(size_t size) > +{ > + unsigned int shift = ilog2(size); > + > + if (shift <= PAGE_SHIFT) > + return 0; > + return (__force fgf_t)((shift - PAGE_SHIFT) << 26); > +} > + > void *filemap_get_entry(struct address_space *mapping, pgoff_t index); > struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index, > fgf_t fgp_flags, gfp_t gfp); > diff --git a/mm/filemap.c b/mm/filemap.c > index 8a669fecfd1c..baafbf324c9f 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -1905,7 +1905,9 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index, > folio_wait_stable(folio); > no_page: > if (!folio && (fgp_flags & FGP_CREAT)) { > + unsigned order = FGF_GET_ORDER(fgp_flags); > int err; > + > if ((fgp_flags & FGP_WRITE) && mapping_can_writeback(mapping)) > gfp |= __GFP_WRITE; > if (fgp_flags & FGP_NOFS) > @@ -1914,26 +1916,44 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index, > gfp &= ~GFP_KERNEL; > gfp |= GFP_NOWAIT | __GFP_NOWARN; > } > - > - folio = filemap_alloc_folio(gfp, 0); > - if (!folio) > - return ERR_PTR(-ENOMEM); > - > if (WARN_ON_ONCE(!(fgp_flags & (FGP_LOCK | FGP_FOR_MMAP)))) > fgp_flags |= FGP_LOCK; > > - /* Init accessed so avoid atomic mark_page_accessed later */ > - if (fgp_flags & FGP_ACCESSED) > - __folio_set_referenced(folio); > + if (!mapping_large_folio_support(mapping)) > + order = 0; > + if (order > MAX_PAGECACHE_ORDER) > + order = MAX_PAGECACHE_ORDER; > + /* If we're not aligned, allocate a smaller folio */ > + if (index & ((1UL << order) - 1)) > + order = __ffs(index); > > - err = filemap_add_folio(mapping, folio, index, gfp); > - if (unlikely(err)) { > + do { > + gfp_t alloc_gfp = gfp; > + > + err = -ENOMEM; > + if (order == 1) > + order = 0; > + if (order > 0) > + alloc_gfp |= __GFP_NORETRY | __GFP_NOWARN; > + folio = filemap_alloc_folio(alloc_gfp, order); > + if (!folio) > + continue; > + > + /* Init accessed so avoid atomic mark_page_accessed later */ > + if (fgp_flags & FGP_ACCESSED) > + __folio_set_referenced(folio); > + > + err = filemap_add_folio(mapping, folio, index, gfp); > + if (!err) > + break; > folio_put(folio); > folio = NULL; > - if (err == -EEXIST) > - goto repeat; > - } > + } while (order-- > 0); > > + if (err == -EEXIST) > + goto repeat; > + if (err) > + return ERR_PTR(err); > /* > * filemap_add_folio locks the page, and for mmap > * we expect an unlocked page. > diff --git a/mm/readahead.c b/mm/readahead.c > index a9c999aa19af..e815c114de21 100644 > --- a/mm/readahead.c > +++ b/mm/readahead.c > @@ -461,19 +461,6 @@ static int try_context_readahead(struct address_space *mapping, > return 1; > } > > -/* > - * There are some parts of the kernel which assume that PMD entries > - * are exactly HPAGE_PMD_ORDER. Those should be fixed, but until then, > - * limit the maximum allocation order to PMD size. I'm not aware of any > - * assumptions about maximum order if THP are disabled, but 8 seems like > - * a good order (that's 1MB if you're using 4kB pages) > - */ > -#ifdef CONFIG_TRANSPARENT_HUGEPAGE > -#define MAX_PAGECACHE_ORDER HPAGE_PMD_ORDER > -#else > -#define MAX_PAGECACHE_ORDER 8 > -#endif > - > static inline int ra_alloc_folio(struct readahead_control *ractl, pgoff_t index, > pgoff_t mark, unsigned int order, gfp_t gfp) > { > -- > 2.39.2 >
On Thu, Jul 13, 2023 at 01:04:39AM -0400, Kent Overstreet wrote: > On Mon, Jul 10, 2023 at 02:02:51PM +0100, Matthew Wilcox (Oracle) wrote: > > Allow callers of __filemap_get_folio() to specify a preferred folio > > order in the FGP flags. This is only honoured in the FGP_CREATE path; > > if there is already a folio in the page cache that covers the index, > > we will return it, no matter what its order is. No create-around is > > attempted; we will only create folios which start at the specified index. > > Unmodified callers will continue to allocate order 0 folios. > > Why not just add an end_index parameter to filemap_get_folio()? I'm reluctant to add more parameters. Aside from the churn, every extra parameter makes the function that little bit harder to use. I like this encoding; users who don't know/care about it get the current default behaviour, and it's a simple addition to the users who do want to care. end_index is particularly tricky ... what if it's lower than index?
On Thu, Jul 13, 2023 at 03:42:18PM +0100, Matthew Wilcox wrote: > On Thu, Jul 13, 2023 at 01:04:39AM -0400, Kent Overstreet wrote: > > On Mon, Jul 10, 2023 at 02:02:51PM +0100, Matthew Wilcox (Oracle) wrote: > > > Allow callers of __filemap_get_folio() to specify a preferred folio > > > order in the FGP flags. This is only honoured in the FGP_CREATE path; > > > if there is already a folio in the page cache that covers the index, > > > we will return it, no matter what its order is. No create-around is > > > attempted; we will only create folios which start at the specified index. > > > Unmodified callers will continue to allocate order 0 folios. > > > > Why not just add an end_index parameter to filemap_get_folio()? > > I'm reluctant to add more parameters. Aside from the churn, every extra > parameter makes the function that little bit harder to use. I like this > encoding; users who don't know/care about it get the current default > behaviour, and it's a simple addition to the users who do want to care. > end_index is particularly tricky ... what if it's lower than index? But we're refactoring all this code (and chaning our thinking) to extents/ranges, not blocks - I'd say end_index is more natural in the long run. Plus, it lets us put the logic for "how big of a folio do we want to allocate" in one place. (end_index < index is just a BUG_ON()).
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h index 911201fc41fc..d87840acbfb2 100644 --- a/include/linux/pagemap.h +++ b/include/linux/pagemap.h @@ -470,6 +470,19 @@ static inline void *detach_page_private(struct page *page) return folio_detach_private(page_folio(page)); } +/* + * There are some parts of the kernel which assume that PMD entries + * are exactly HPAGE_PMD_ORDER. Those should be fixed, but until then, + * limit the maximum allocation order to PMD size. I'm not aware of any + * assumptions about maximum order if THP are disabled, but 8 seems like + * a good order (that's 1MB if you're using 4kB pages) + */ +#ifdef CONFIG_TRANSPARENT_HUGEPAGE +#define MAX_PAGECACHE_ORDER HPAGE_PMD_ORDER +#else +#define MAX_PAGECACHE_ORDER 8 +#endif + #ifdef CONFIG_NUMA struct folio *filemap_alloc_folio(gfp_t gfp, unsigned int order); #else @@ -535,9 +548,30 @@ typedef unsigned int __bitwise fgf_t; #define FGP_NOWAIT ((__force fgf_t)0x00000020) #define FGP_FOR_MMAP ((__force fgf_t)0x00000040) #define FGP_STABLE ((__force fgf_t)0x00000080) +#define FGF_GET_ORDER(fgf) (((__force unsigned)fgf) >> 26) /* top 6 bits */ #define FGP_WRITEBEGIN (FGP_LOCK | FGP_WRITE | FGP_CREAT | FGP_STABLE) +/** + * fgf_set_order - Encode a length in the fgf_t flags. + * @size: The suggested size of the folio to create. + * + * The caller of __filemap_get_folio() can use this to suggest a preferred + * size for the folio that is created. If there is already a folio at + * the index, it will be returned, no matter what its size. If a folio + * is freshly created, it may be of a different size than requested + * due to alignment constraints, memory pressure, or the presence of + * other folios at nearby indices. + */ +static inline fgf_t fgf_set_order(size_t size) +{ + unsigned int shift = ilog2(size); + + if (shift <= PAGE_SHIFT) + return 0; + return (__force fgf_t)((shift - PAGE_SHIFT) << 26); +} + void *filemap_get_entry(struct address_space *mapping, pgoff_t index); struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index, fgf_t fgp_flags, gfp_t gfp); diff --git a/mm/filemap.c b/mm/filemap.c index 8a669fecfd1c..baafbf324c9f 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -1905,7 +1905,9 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index, folio_wait_stable(folio); no_page: if (!folio && (fgp_flags & FGP_CREAT)) { + unsigned order = FGF_GET_ORDER(fgp_flags); int err; + if ((fgp_flags & FGP_WRITE) && mapping_can_writeback(mapping)) gfp |= __GFP_WRITE; if (fgp_flags & FGP_NOFS) @@ -1914,26 +1916,44 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index, gfp &= ~GFP_KERNEL; gfp |= GFP_NOWAIT | __GFP_NOWARN; } - - folio = filemap_alloc_folio(gfp, 0); - if (!folio) - return ERR_PTR(-ENOMEM); - if (WARN_ON_ONCE(!(fgp_flags & (FGP_LOCK | FGP_FOR_MMAP)))) fgp_flags |= FGP_LOCK; - /* Init accessed so avoid atomic mark_page_accessed later */ - if (fgp_flags & FGP_ACCESSED) - __folio_set_referenced(folio); + if (!mapping_large_folio_support(mapping)) + order = 0; + if (order > MAX_PAGECACHE_ORDER) + order = MAX_PAGECACHE_ORDER; + /* If we're not aligned, allocate a smaller folio */ + if (index & ((1UL << order) - 1)) + order = __ffs(index); - err = filemap_add_folio(mapping, folio, index, gfp); - if (unlikely(err)) { + do { + gfp_t alloc_gfp = gfp; + + err = -ENOMEM; + if (order == 1) + order = 0; + if (order > 0) + alloc_gfp |= __GFP_NORETRY | __GFP_NOWARN; + folio = filemap_alloc_folio(alloc_gfp, order); + if (!folio) + continue; + + /* Init accessed so avoid atomic mark_page_accessed later */ + if (fgp_flags & FGP_ACCESSED) + __folio_set_referenced(folio); + + err = filemap_add_folio(mapping, folio, index, gfp); + if (!err) + break; folio_put(folio); folio = NULL; - if (err == -EEXIST) - goto repeat; - } + } while (order-- > 0); + if (err == -EEXIST) + goto repeat; + if (err) + return ERR_PTR(err); /* * filemap_add_folio locks the page, and for mmap * we expect an unlocked page. diff --git a/mm/readahead.c b/mm/readahead.c index a9c999aa19af..e815c114de21 100644 --- a/mm/readahead.c +++ b/mm/readahead.c @@ -461,19 +461,6 @@ static int try_context_readahead(struct address_space *mapping, return 1; } -/* - * There are some parts of the kernel which assume that PMD entries - * are exactly HPAGE_PMD_ORDER. Those should be fixed, but until then, - * limit the maximum allocation order to PMD size. I'm not aware of any - * assumptions about maximum order if THP are disabled, but 8 seems like - * a good order (that's 1MB if you're using 4kB pages) - */ -#ifdef CONFIG_TRANSPARENT_HUGEPAGE -#define MAX_PAGECACHE_ORDER HPAGE_PMD_ORDER -#else -#define MAX_PAGECACHE_ORDER 8 -#endif - static inline int ra_alloc_folio(struct readahead_control *ractl, pgoff_t index, pgoff_t mark, unsigned int order, gfp_t gfp) {