| Message ID | 20210622121551.3398730-47-willy@infradead.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Folio-enabling the page cache | expand |
On Tue, Jun 22, 2021 at 01:15:51PM +0100, Matthew Wilcox (Oracle) wrote: > Allow filemap_get_folio() to wait for writeback to complete (if the > filesystem wants that behaviour). This is the folio equivalent of > grab_cache_page_write_begin(), which is moved into the folio-compat > file as a reminder to migrate all the code using it. This paves the > way for getting rid of AOP_FLAG_NOFS once grab_cache_page_write_begin() > is removed. We actually should kill FGP_NOFS as well by switching everything over to memalloc_nofs_{save, restore} eventually, given how error prone all these manual flags settings are. > diff --git a/mm/folio-compat.c b/mm/folio-compat.c > index 78365eaee7d3..206bedd621d0 100644 > --- a/mm/folio-compat.c > +++ b/mm/folio-compat.c > @@ -115,6 +115,7 @@ void lru_cache_add(struct page *page) > } > EXPORT_SYMBOL(lru_cache_add); > > +noinline > struct page *pagecache_get_page(struct address_space *mapping, pgoff_t index, > int fgp_flags, gfp_t gfp) How did that sneak in here? Otherwise: Reviewed-by: Christoph Hellwig <hch@lst.de>
On Wed, Jun 23, 2021 at 01:43:30PM +0200, Christoph Hellwig wrote: > On Tue, Jun 22, 2021 at 01:15:51PM +0100, Matthew Wilcox (Oracle) wrote: > > Allow filemap_get_folio() to wait for writeback to complete (if the > > filesystem wants that behaviour). This is the folio equivalent of > > grab_cache_page_write_begin(), which is moved into the folio-compat > > file as a reminder to migrate all the code using it. This paves the > > way for getting rid of AOP_FLAG_NOFS once grab_cache_page_write_begin() > > is removed. > > We actually should kill FGP_NOFS as well by switching everything over > to memalloc_nofs_{save, restore} eventually, given how error prone > all these manual flags settings are. Well, yes, but it's been four years and we still have over 1100 uses of GFP_NOFS. Until someone takes on that Augean Stables, we're going to need FGP_NOFS. I added that context to the readahead path in f2c817bed58d, but of course that doesn't let me remove any uses of GFP_NOFS. > > diff --git a/mm/folio-compat.c b/mm/folio-compat.c > > index 78365eaee7d3..206bedd621d0 100644 > > --- a/mm/folio-compat.c > > +++ b/mm/folio-compat.c > > @@ -115,6 +115,7 @@ void lru_cache_add(struct page *page) > > } > > EXPORT_SYMBOL(lru_cache_add); > > > > +noinline > > struct page *pagecache_get_page(struct address_space *mapping, pgoff_t index, > > int fgp_flags, gfp_t gfp) > > How did that sneak in here? Without it, pagecache_get_page() gets inlined by grab_cache_page_write_begin() which is just too much code.
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h index 669bdebe2861..865b7784e99b 100644 --- a/include/linux/pagemap.h +++ b/include/linux/pagemap.h @@ -301,6 +301,7 @@ pgoff_t page_cache_prev_miss(struct address_space *mapping, #define FGP_FOR_MMAP 0x00000040 #define FGP_HEAD 0x00000080 #define FGP_ENTRY 0x00000100 +#define FGP_STABLE 0x00000200 struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index, int fgp_flags, gfp_t gfp); diff --git a/mm/filemap.c b/mm/filemap.c index e431461279c3..4951fe8787d8 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -1817,6 +1817,7 @@ static struct folio *mapping_get_entry(struct address_space *mapping, * * %FGP_WRITE - The page will be written to by the caller. * * %FGP_NOFS - __GFP_FS will get cleared in gfp. * * %FGP_NOWAIT - Don't get blocked by page lock. + * * %FGP_STABLE - Wait for the folio to be stable (finished writeback) * * If %FGP_LOCK or %FGP_CREAT are specified then the function may sleep even * if the %GFP flags specified for %FGP_CREAT are atomic. @@ -1867,6 +1868,8 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index, folio_clear_idle_flag(folio); } + if (fgp_flags & FGP_STABLE) + folio_wait_stable(folio); no_page: if (!folio && (fgp_flags & FGP_CREAT)) { int err; @@ -3590,28 +3593,6 @@ generic_file_direct_write(struct kiocb *iocb, struct iov_iter *from) } EXPORT_SYMBOL(generic_file_direct_write); -/* - * Find or create a page at the given pagecache position. Return the locked - * page. This function is specifically for buffered writes. - */ -struct page *grab_cache_page_write_begin(struct address_space *mapping, - pgoff_t index, unsigned flags) -{ - struct page *page; - int fgp_flags = FGP_LOCK|FGP_WRITE|FGP_CREAT; - - if (flags & AOP_FLAG_NOFS) - fgp_flags |= FGP_NOFS; - - page = pagecache_get_page(mapping, index, fgp_flags, - mapping_gfp_mask(mapping)); - if (page) - wait_for_stable_page(page); - - return page; -} -EXPORT_SYMBOL(grab_cache_page_write_begin); - ssize_t generic_perform_write(struct file *file, struct iov_iter *i, loff_t pos) { diff --git a/mm/folio-compat.c b/mm/folio-compat.c index 78365eaee7d3..206bedd621d0 100644 --- a/mm/folio-compat.c +++ b/mm/folio-compat.c @@ -115,6 +115,7 @@ void lru_cache_add(struct page *page) } EXPORT_SYMBOL(lru_cache_add); +noinline struct page *pagecache_get_page(struct address_space *mapping, pgoff_t index, int fgp_flags, gfp_t gfp) { @@ -126,3 +127,15 @@ struct page *pagecache_get_page(struct address_space *mapping, pgoff_t index, return folio_file_page(folio, index); } EXPORT_SYMBOL(pagecache_get_page); + +struct page *grab_cache_page_write_begin(struct address_space *mapping, + pgoff_t index, unsigned flags) +{ + unsigned fgp_flags = FGP_LOCK | FGP_WRITE | FGP_CREAT | FGP_STABLE; + + if (flags & AOP_FLAG_NOFS) + fgp_flags |= FGP_NOFS; + return pagecache_get_page(mapping, index, fgp_flags, + mapping_gfp_mask(mapping)); +} +EXPORT_SYMBOL(grab_cache_page_write_begin);
Allow filemap_get_folio() to wait for writeback to complete (if the filesystem wants that behaviour). This is the folio equivalent of grab_cache_page_write_begin(), which is moved into the folio-compat file as a reminder to migrate all the code using it. This paves the way for getting rid of AOP_FLAG_NOFS once grab_cache_page_write_begin() is removed. Kernel grows by 11 bytes. filemap_get_folio() grows by 33 bytes but grab_cache_page_write_begin() shrinks by 22 bytes to make up for it. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- include/linux/pagemap.h | 1 + mm/filemap.c | 25 +++---------------------- mm/folio-compat.c | 13 +++++++++++++ 3 files changed, 17 insertions(+), 22 deletions(-)