| Message ID | 20210224164455.1096727-2-axboe@kernel.dk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Improve IOCB_NOWAIT O_DIRECT reads | expand |
On Wed, Feb 24, 2021 at 09:44:53AM -0700, Jens Axboe wrote: > +++ b/include/linux/fs.h > @@ -2633,6 +2633,8 @@ static inline int filemap_fdatawait(struct address_space *mapping) > > extern bool filemap_range_has_page(struct address_space *, loff_t lstart, > loff_t lend); > +extern bool filemap_range_needs_writeback(struct address_space *, > + loff_t lstart, loff_t lend); > extern int filemap_write_and_wait_range(struct address_space *mapping, > loff_t lstart, loff_t lend); > extern int __filemap_fdatawrite_range(struct address_space *mapping, These prototypes should all be in pagemap.h, not fs.h. I can do a followup patch if you don't want to do it as part of this set. Also we're deprecating the use of 'extern' for prototypes. Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
On 2/24/21 9:50 AM, Matthew Wilcox wrote: > On Wed, Feb 24, 2021 at 09:44:53AM -0700, Jens Axboe wrote: >> +++ b/include/linux/fs.h >> @@ -2633,6 +2633,8 @@ static inline int filemap_fdatawait(struct address_space *mapping) >> >> extern bool filemap_range_has_page(struct address_space *, loff_t lstart, >> loff_t lend); >> +extern bool filemap_range_needs_writeback(struct address_space *, >> + loff_t lstart, loff_t lend); >> extern int filemap_write_and_wait_range(struct address_space *mapping, >> loff_t lstart, loff_t lend); >> extern int __filemap_fdatawrite_range(struct address_space *mapping, > > These prototypes should all be in pagemap.h, not fs.h. I can do > a followup patch if you don't want to do it as part of this set. > Also we're deprecating the use of 'extern' for prototypes. Agree on both of those, but that should probably be a separate patch for both of those. extern is just following the existing style, and fs.h is the existing location... > Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org> Thanks!
On Wed 24-02-21 09:44:53, Jens Axboe wrote: > For O_DIRECT reads/writes, we check if we need to issue a call to > filemap_write_and_wait_range() to issue and/or wait for writeback for any > page in the given range. The existing mechanism just checks for a page in > the range, which is suboptimal for IOCB_NOWAIT as we'll fallback to the > slow path (and needing retry) if there's just a clean page cache page in > the range. > > Provide filemap_range_needs_writeback() which tries a little harder to > check if we actually need to issue and/or wait for writeback in the > range. > > Signed-off-by: Jens Axboe <axboe@kernel.dk> Looks good to me. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > include/linux/fs.h | 2 ++ > mm/filemap.c | 43 +++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 45 insertions(+) > > diff --git a/include/linux/fs.h b/include/linux/fs.h > index 6d8b1e7337e4..4925275e6365 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -2633,6 +2633,8 @@ static inline int filemap_fdatawait(struct address_space *mapping) > > extern bool filemap_range_has_page(struct address_space *, loff_t lstart, > loff_t lend); > +extern bool filemap_range_needs_writeback(struct address_space *, > + loff_t lstart, loff_t lend); > extern int filemap_write_and_wait_range(struct address_space *mapping, > loff_t lstart, loff_t lend); > extern int __filemap_fdatawrite_range(struct address_space *mapping, > diff --git a/mm/filemap.c b/mm/filemap.c > index 6ff2a3fb0dc7..13338f877677 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -635,6 +635,49 @@ static bool mapping_needs_writeback(struct address_space *mapping) > return mapping->nrpages; > } > > +/** > + * filemap_range_needs_writeback - check if range potentially needs writeback > + * @mapping: address space within which to check > + * @start_byte: offset in bytes where the range starts > + * @end_byte: offset in bytes where the range ends (inclusive) > + * > + * Find at least one page in the range supplied, usually used to check if > + * direct writing in this range will trigger a writeback. Used by O_DIRECT > + * read/write with IOCB_NOWAIT, to see if the caller needs to do > + * filemap_write_and_wait_range() before proceeding. > + * > + * Return: %true if the caller should do filemap_write_and_wait_range() before > + * doing O_DIRECT to a page in this range, %false otherwise. > + */ > +bool filemap_range_needs_writeback(struct address_space *mapping, > + loff_t start_byte, loff_t end_byte) > +{ > + XA_STATE(xas, &mapping->i_pages, start_byte >> PAGE_SHIFT); > + pgoff_t max = end_byte >> PAGE_SHIFT; > + struct page *page; > + > + if (!mapping_needs_writeback(mapping)) > + return false; > + if (!mapping_tagged(mapping, PAGECACHE_TAG_DIRTY) && > + !mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK)) > + return false; > + if (end_byte < start_byte) > + return false; > + > + rcu_read_lock(); > + xas_for_each(&xas, page, max) { > + if (xas_retry(&xas, page)) > + continue; > + if (xa_is_value(page)) > + continue; > + if (PageDirty(page) || PageLocked(page) || PageWriteback(page)) > + break; > + } > + rcu_read_unlock(); > + return page != NULL; > +} > +EXPORT_SYMBOL_GPL(filemap_range_needs_writeback); > + > /** > * filemap_write_and_wait_range - write out & wait on a file range > * @mapping: the address_space for the pages > -- > 2.30.0 >
diff --git a/include/linux/fs.h b/include/linux/fs.h index 6d8b1e7337e4..4925275e6365 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -2633,6 +2633,8 @@ static inline int filemap_fdatawait(struct address_space *mapping) extern bool filemap_range_has_page(struct address_space *, loff_t lstart, loff_t lend); +extern bool filemap_range_needs_writeback(struct address_space *, + loff_t lstart, loff_t lend); extern int filemap_write_and_wait_range(struct address_space *mapping, loff_t lstart, loff_t lend); extern int __filemap_fdatawrite_range(struct address_space *mapping, diff --git a/mm/filemap.c b/mm/filemap.c index 6ff2a3fb0dc7..13338f877677 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -635,6 +635,49 @@ static bool mapping_needs_writeback(struct address_space *mapping) return mapping->nrpages; } +/** + * filemap_range_needs_writeback - check if range potentially needs writeback + * @mapping: address space within which to check + * @start_byte: offset in bytes where the range starts + * @end_byte: offset in bytes where the range ends (inclusive) + * + * Find at least one page in the range supplied, usually used to check if + * direct writing in this range will trigger a writeback. Used by O_DIRECT + * read/write with IOCB_NOWAIT, to see if the caller needs to do + * filemap_write_and_wait_range() before proceeding. + * + * Return: %true if the caller should do filemap_write_and_wait_range() before + * doing O_DIRECT to a page in this range, %false otherwise. + */ +bool filemap_range_needs_writeback(struct address_space *mapping, + loff_t start_byte, loff_t end_byte) +{ + XA_STATE(xas, &mapping->i_pages, start_byte >> PAGE_SHIFT); + pgoff_t max = end_byte >> PAGE_SHIFT; + struct page *page; + + if (!mapping_needs_writeback(mapping)) + return false; + if (!mapping_tagged(mapping, PAGECACHE_TAG_DIRTY) && + !mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK)) + return false; + if (end_byte < start_byte) + return false; + + rcu_read_lock(); + xas_for_each(&xas, page, max) { + if (xas_retry(&xas, page)) + continue; + if (xa_is_value(page)) + continue; + if (PageDirty(page) || PageLocked(page) || PageWriteback(page)) + break; + } + rcu_read_unlock(); + return page != NULL; +} +EXPORT_SYMBOL_GPL(filemap_range_needs_writeback); + /** * filemap_write_and_wait_range - write out & wait on a file range * @mapping: the address_space for the pages
For O_DIRECT reads/writes, we check if we need to issue a call to filemap_write_and_wait_range() to issue and/or wait for writeback for any page in the given range. The existing mechanism just checks for a page in the range, which is suboptimal for IOCB_NOWAIT as we'll fallback to the slow path (and needing retry) if there's just a clean page cache page in the range. Provide filemap_range_needs_writeback() which tries a little harder to check if we actually need to issue and/or wait for writeback in the range. Signed-off-by: Jens Axboe <axboe@kernel.dk> --- include/linux/fs.h | 2 ++ mm/filemap.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+)