| Message ID | 20211203153829.298893-2-axboe@kernel.dk (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Avoid unnecessary indirect calls for bdev dio | expand |
On Fri, Dec 03, 2021 at 08:38:28AM -0700, Jens Axboe wrote: > +++ b/include/linux/fs.h fs.h is the wrong place for these functions; they're pagecache functionality, so they should be in pagemap.h. > +/* Returns true if writeback might be needed or already in progress. */ > +static inline bool mapping_needs_writeback(struct address_space *mapping) > +{ > + return mapping->nrpages; > +} I don't like this function -- mapping_needs_writeback says to me that it tests a flag in mapping->flags. Plus, it does exactly the same thing as !mapping_empty(), so perhaps ... > +static inline bool filemap_range_needs_writeback(struct address_space *mapping, > + loff_t start_byte, > + loff_t end_byte) > +{ > + if (!mapping_needs_writeback(mapping)) > + return false; just make this if (mapping_empty(mapping)) return false; Other than that, no objections to making this static inline.
On 12/3/21 9:16 AM, Matthew Wilcox wrote: > On Fri, Dec 03, 2021 at 08:38:28AM -0700, Jens Axboe wrote: >> +++ b/include/linux/fs.h > > fs.h is the wrong place for these functions; they're pagecache > functionality, so they should be in pagemap.h. > >> +/* Returns true if writeback might be needed or already in progress. */ >> +static inline bool mapping_needs_writeback(struct address_space *mapping) >> +{ >> + return mapping->nrpages; >> +} > > I don't like this function -- mapping_needs_writeback says to me that it > tests a flag in mapping->flags. Plus, it does exactly the same thing as > !mapping_empty(), so perhaps ... > >> +static inline bool filemap_range_needs_writeback(struct address_space *mapping, >> + loff_t start_byte, >> + loff_t end_byte) >> +{ >> + if (!mapping_needs_writeback(mapping)) >> + return false; > > just make this > if (mapping_empty(mapping)) > return false; > > Other than that, no objections to making this static inline. Good idea, I'll make that change.
On 12/3/21 9:24 AM, Jens Axboe wrote: > On 12/3/21 9:16 AM, Matthew Wilcox wrote: >> On Fri, Dec 03, 2021 at 08:38:28AM -0700, Jens Axboe wrote: >>> +++ b/include/linux/fs.h >> >> fs.h is the wrong place for these functions; they're pagecache >> functionality, so they should be in pagemap.h. >> >>> +/* Returns true if writeback might be needed or already in progress. */ >>> +static inline bool mapping_needs_writeback(struct address_space *mapping) >>> +{ >>> + return mapping->nrpages; >>> +} >> >> I don't like this function -- mapping_needs_writeback says to me that it >> tests a flag in mapping->flags. Plus, it does exactly the same thing as >> !mapping_empty(), so perhaps ... >> >>> +static inline bool filemap_range_needs_writeback(struct address_space *mapping, >>> + loff_t start_byte, >>> + loff_t end_byte) >>> +{ >>> + if (!mapping_needs_writeback(mapping)) >>> + return false; >> >> just make this >> if (mapping_empty(mapping)) >> return false; >> >> Other than that, no objections to making this static inline. > > Good idea, I'll make that change. That does introduce a dependency from fs.h -> pagemap.h which isn't trivially resolvable... What if we just rename the above funciton to mapping_has_pages() or something instead?
On 12/3/21 9:31 AM, Jens Axboe wrote: > On 12/3/21 9:24 AM, Jens Axboe wrote: >> On 12/3/21 9:16 AM, Matthew Wilcox wrote: >>> On Fri, Dec 03, 2021 at 08:38:28AM -0700, Jens Axboe wrote: >>>> +++ b/include/linux/fs.h >>> >>> fs.h is the wrong place for these functions; they're pagecache >>> functionality, so they should be in pagemap.h. >>> >>>> +/* Returns true if writeback might be needed or already in progress. */ >>>> +static inline bool mapping_needs_writeback(struct address_space *mapping) >>>> +{ >>>> + return mapping->nrpages; >>>> +} >>> >>> I don't like this function -- mapping_needs_writeback says to me that it >>> tests a flag in mapping->flags. Plus, it does exactly the same thing as >>> !mapping_empty(), so perhaps ... >>> >>>> +static inline bool filemap_range_needs_writeback(struct address_space *mapping, >>>> + loff_t start_byte, >>>> + loff_t end_byte) >>>> +{ >>>> + if (!mapping_needs_writeback(mapping)) >>>> + return false; >>> >>> just make this >>> if (mapping_empty(mapping)) >>> return false; >>> >>> Other than that, no objections to making this static inline. >> >> Good idea, I'll make that change. > > That does introduce a dependency from fs.h -> pagemap.h which isn't trivially > resolvable... > > What if we just rename the above funciton to mapping_has_pages() or something > instead? Or just drop the helper, to be honest. There are more tests for mapping->nrpages right now than there are callers of this silly little helper.
On 12/3/21 9:35 AM, Jens Axboe wrote: > On 12/3/21 9:31 AM, Jens Axboe wrote: >> On 12/3/21 9:24 AM, Jens Axboe wrote: >>> On 12/3/21 9:16 AM, Matthew Wilcox wrote: >>>> On Fri, Dec 03, 2021 at 08:38:28AM -0700, Jens Axboe wrote: >>>>> +++ b/include/linux/fs.h >>>> >>>> fs.h is the wrong place for these functions; they're pagecache >>>> functionality, so they should be in pagemap.h. >>>> >>>>> +/* Returns true if writeback might be needed or already in progress. */ >>>>> +static inline bool mapping_needs_writeback(struct address_space *mapping) >>>>> +{ >>>>> + return mapping->nrpages; >>>>> +} >>>> >>>> I don't like this function -- mapping_needs_writeback says to me that it >>>> tests a flag in mapping->flags. Plus, it does exactly the same thing as >>>> !mapping_empty(), so perhaps ... >>>> >>>>> +static inline bool filemap_range_needs_writeback(struct address_space *mapping, >>>>> + loff_t start_byte, >>>>> + loff_t end_byte) >>>>> +{ >>>>> + if (!mapping_needs_writeback(mapping)) >>>>> + return false; >>>> >>>> just make this >>>> if (mapping_empty(mapping)) >>>> return false; >>>> >>>> Other than that, no objections to making this static inline. >>> >>> Good idea, I'll make that change. >> >> That does introduce a dependency from fs.h -> pagemap.h which isn't trivially >> resolvable... >> >> What if we just rename the above funciton to mapping_has_pages() or something >> instead? > > Or just drop the helper, to be honest. There are more tests for > mapping->nrpages right now than there are callers of this silly little > helper. Like this: commit 80d0d63df336376f53375c98703bcae0ec50d26b Author: Jens Axboe <axboe@kernel.dk> Date: Thu Oct 28 08:47:05 2021 -0600 mm: move filemap_range_needs_writeback() into header No functional changes in this patch, just in preparation for efficiently calling this light function from the block O_DIRECT handling. Signed-off-by: Jens Axboe <axboe@kernel.dk> diff --git a/include/linux/fs.h b/include/linux/fs.h index bbf812ce89a8..11a37adc2520 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -2845,6 +2845,35 @@ static inline int filemap_fdatawait(struct address_space *mapping) return filemap_fdatawait_range(mapping, 0, LLONG_MAX); } +bool filemap_range_has_writeback(struct address_space *mapping, + loff_t start_byte, loff_t end_byte); + +/** + * 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. + */ +static inline bool filemap_range_needs_writeback(struct address_space *mapping, + loff_t start_byte, + loff_t end_byte) +{ + if (!mapping->nrpages) + return false; + if (!mapping_tagged(mapping, PAGECACHE_TAG_DIRTY) && + !mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK)) + return false; + return filemap_range_has_writeback(mapping, start_byte, end_byte); +} + extern bool filemap_range_has_page(struct address_space *, loff_t lstart, loff_t lend); extern bool filemap_range_needs_writeback(struct address_space *, diff --git a/mm/filemap.c b/mm/filemap.c index daa0e23a6ee6..655c9eec06b3 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -646,8 +646,8 @@ static bool mapping_needs_writeback(struct address_space *mapping) return mapping->nrpages; } -static bool filemap_range_has_writeback(struct address_space *mapping, - loff_t start_byte, loff_t end_byte) +bool filemap_range_has_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; @@ -667,34 +667,8 @@ static bool filemap_range_has_writeback(struct address_space *mapping, } rcu_read_unlock(); return page != NULL; - -} - -/** - * 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) -{ - if (!mapping_needs_writeback(mapping)) - return false; - if (!mapping_tagged(mapping, PAGECACHE_TAG_DIRTY) && - !mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK)) - return false; - return filemap_range_has_writeback(mapping, start_byte, end_byte); } -EXPORT_SYMBOL_GPL(filemap_range_needs_writeback); +EXPORT_SYMBOL_GPL(filemap_range_has_writeback); /** * filemap_write_and_wait_range - write out & wait on a file range
On Fri, Dec 03, 2021 at 09:38:25AM -0700, Jens Axboe wrote: > On 12/3/21 9:35 AM, Jens Axboe wrote: > > On 12/3/21 9:31 AM, Jens Axboe wrote: > >> On 12/3/21 9:24 AM, Jens Axboe wrote: > >>> On 12/3/21 9:16 AM, Matthew Wilcox wrote: > >>>> On Fri, Dec 03, 2021 at 08:38:28AM -0700, Jens Axboe wrote: > >>>>> +++ b/include/linux/fs.h > >>>> > >>>> fs.h is the wrong place for these functions; they're pagecache > >>>> functionality, so they should be in pagemap.h. I think you missed this ^^^ > >> That does introduce a dependency from fs.h -> pagemap.h which isn't trivially > >> resolvable... > >> > >> What if we just rename the above funciton to mapping_has_pages() or something > >> instead? > > > > Or just drop the helper, to be honest. There are more tests for > > mapping->nrpages right now than there are callers of this silly little > > helper. > > Like this: I'm happy with this, if you just move it to pagemap.h > > commit 80d0d63df336376f53375c98703bcae0ec50d26b > Author: Jens Axboe <axboe@kernel.dk> > Date: Thu Oct 28 08:47:05 2021 -0600 > > mm: move filemap_range_needs_writeback() into header > > No functional changes in this patch, just in preparation for efficiently > calling this light function from the block O_DIRECT handling. > > Signed-off-by: Jens Axboe <axboe@kernel.dk> > > diff --git a/include/linux/fs.h b/include/linux/fs.h > index bbf812ce89a8..11a37adc2520 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -2845,6 +2845,35 @@ static inline int filemap_fdatawait(struct address_space *mapping) > return filemap_fdatawait_range(mapping, 0, LLONG_MAX); > } > > +bool filemap_range_has_writeback(struct address_space *mapping, > + loff_t start_byte, loff_t end_byte); > + > +/** > + * 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. > + */ > +static inline bool filemap_range_needs_writeback(struct address_space *mapping, > + loff_t start_byte, > + loff_t end_byte) > +{ > + if (!mapping->nrpages) > + return false; > + if (!mapping_tagged(mapping, PAGECACHE_TAG_DIRTY) && > + !mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK)) > + return false; > + return filemap_range_has_writeback(mapping, start_byte, end_byte); > +} > + > extern bool filemap_range_has_page(struct address_space *, loff_t lstart, > loff_t lend); > extern bool filemap_range_needs_writeback(struct address_space *, > diff --git a/mm/filemap.c b/mm/filemap.c > index daa0e23a6ee6..655c9eec06b3 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -646,8 +646,8 @@ static bool mapping_needs_writeback(struct address_space *mapping) > return mapping->nrpages; > } > > -static bool filemap_range_has_writeback(struct address_space *mapping, > - loff_t start_byte, loff_t end_byte) > +bool filemap_range_has_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; > @@ -667,34 +667,8 @@ static bool filemap_range_has_writeback(struct address_space *mapping, > } > rcu_read_unlock(); > return page != NULL; > - > -} > - > -/** > - * 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) > -{ > - if (!mapping_needs_writeback(mapping)) > - return false; > - if (!mapping_tagged(mapping, PAGECACHE_TAG_DIRTY) && > - !mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK)) > - return false; > - return filemap_range_has_writeback(mapping, start_byte, end_byte); > } > -EXPORT_SYMBOL_GPL(filemap_range_needs_writeback); > +EXPORT_SYMBOL_GPL(filemap_range_has_writeback); > > /** > * filemap_write_and_wait_range - write out & wait on a file range > > > -- > Jens Axboe >
On 12/3/21 10:46 AM, Matthew Wilcox wrote: > On Fri, Dec 03, 2021 at 09:38:25AM -0700, Jens Axboe wrote: >> On 12/3/21 9:35 AM, Jens Axboe wrote: >>> On 12/3/21 9:31 AM, Jens Axboe wrote: >>>> On 12/3/21 9:24 AM, Jens Axboe wrote: >>>>> On 12/3/21 9:16 AM, Matthew Wilcox wrote: >>>>>> On Fri, Dec 03, 2021 at 08:38:28AM -0700, Jens Axboe wrote: >>>>>>> +++ b/include/linux/fs.h >>>>>> >>>>>> fs.h is the wrong place for these functions; they're pagecache >>>>>> functionality, so they should be in pagemap.h. > > I think you missed this ^^^ I did... That would mean moving some more of the declarations too. Prep patch or just part of this patch? >>>> That does introduce a dependency from fs.h -> pagemap.h which isn't trivially >>>> resolvable... >>>> >>>> What if we just rename the above funciton to mapping_has_pages() or something >>>> instead? >>> >>> Or just drop the helper, to be honest. There are more tests for >>> mapping->nrpages right now than there are callers of this silly little >>> helper. >> >> Like this: > > I'm happy with this, if you just move it to pagemap.h OK, I'll try it out.
On 12/3/21 10:57 AM, Jens Axboe wrote: >> I'm happy with this, if you just move it to pagemap.h > > OK, I'll try it out. Wasn't too bad at all, actually just highlighted that I missed removing the previous declaration of filemap_range_needs_writeback() in fs.h I'll do a full compile and test, but this seems sane. commit 63c6b3846b77041d239d5b5b5a907b5c82a21c4c Author: Jens Axboe <axboe@kernel.dk> Date: Thu Oct 28 08:47:05 2021 -0600 mm: move filemap_range_needs_writeback() into header No functional changes in this patch, just in preparation for efficiently calling this light function from the block O_DIRECT handling. Signed-off-by: Jens Axboe <axboe@kernel.dk> diff --git a/include/linux/fs.h b/include/linux/fs.h index bbf812ce89a8..6b8dc1a78df6 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -2847,8 +2847,6 @@ 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/include/linux/pagemap.h b/include/linux/pagemap.h index 605246452305..274a0710f2c5 100644 --- a/include/linux/pagemap.h +++ b/include/linux/pagemap.h @@ -963,6 +963,35 @@ static inline int add_to_page_cache(struct page *page, int __filemap_add_folio(struct address_space *mapping, struct folio *folio, pgoff_t index, gfp_t gfp, void **shadowp); +bool filemap_range_has_writeback(struct address_space *mapping, + loff_t start_byte, loff_t end_byte); + +/** + * 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. + */ +static inline bool filemap_range_needs_writeback(struct address_space *mapping, + loff_t start_byte, + loff_t end_byte) +{ + if (!mapping->nrpages) + return false; + if (!mapping_tagged(mapping, PAGECACHE_TAG_DIRTY) && + !mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK)) + return false; + return filemap_range_has_writeback(mapping, start_byte, end_byte); +} + /** * struct readahead_control - Describes a readahead request. * diff --git a/mm/filemap.c b/mm/filemap.c index daa0e23a6ee6..655c9eec06b3 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -646,8 +646,8 @@ static bool mapping_needs_writeback(struct address_space *mapping) return mapping->nrpages; } -static bool filemap_range_has_writeback(struct address_space *mapping, - loff_t start_byte, loff_t end_byte) +bool filemap_range_has_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; @@ -667,34 +667,8 @@ static bool filemap_range_has_writeback(struct address_space *mapping, } rcu_read_unlock(); return page != NULL; - -} - -/** - * 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) -{ - if (!mapping_needs_writeback(mapping)) - return false; - if (!mapping_tagged(mapping, PAGECACHE_TAG_DIRTY) && - !mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK)) - return false; - return filemap_range_has_writeback(mapping, start_byte, end_byte); } -EXPORT_SYMBOL_GPL(filemap_range_needs_writeback); +EXPORT_SYMBOL_GPL(filemap_range_has_writeback); /** * filemap_write_and_wait_range - write out & wait on a file range
On Fri, Dec 03, 2021 at 11:01:14AM -0700, Jens Axboe wrote: > On 12/3/21 10:57 AM, Jens Axboe wrote: > >> I'm happy with this, if you just move it to pagemap.h > > > > OK, I'll try it out. > > Wasn't too bad at all, actually just highlighted that I missed removing > the previous declaration of filemap_range_needs_writeback() in fs.h > I'll do a full compile and test, but this seems sane. > > commit 63c6b3846b77041d239d5b5b5a907b5c82a21c4c > Author: Jens Axboe <axboe@kernel.dk> > Date: Thu Oct 28 08:47:05 2021 -0600 > > mm: move filemap_range_needs_writeback() into header > > No functional changes in this patch, just in preparation for efficiently > calling this light function from the block O_DIRECT handling. > > Signed-off-by: Jens Axboe <axboe@kernel.dk> Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org> > diff --git a/include/linux/fs.h b/include/linux/fs.h > index bbf812ce89a8..6b8dc1a78df6 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -2847,8 +2847,6 @@ 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/include/linux/pagemap.h b/include/linux/pagemap.h > index 605246452305..274a0710f2c5 100644 > --- a/include/linux/pagemap.h > +++ b/include/linux/pagemap.h > @@ -963,6 +963,35 @@ static inline int add_to_page_cache(struct page *page, > int __filemap_add_folio(struct address_space *mapping, struct folio *folio, > pgoff_t index, gfp_t gfp, void **shadowp); > > +bool filemap_range_has_writeback(struct address_space *mapping, > + loff_t start_byte, loff_t end_byte); > + > +/** > + * 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. > + */ > +static inline bool filemap_range_needs_writeback(struct address_space *mapping, > + loff_t start_byte, > + loff_t end_byte) > +{ > + if (!mapping->nrpages) > + return false; > + if (!mapping_tagged(mapping, PAGECACHE_TAG_DIRTY) && > + !mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK)) > + return false; > + return filemap_range_has_writeback(mapping, start_byte, end_byte); > +} > + > /** > * struct readahead_control - Describes a readahead request. > * > diff --git a/mm/filemap.c b/mm/filemap.c > index daa0e23a6ee6..655c9eec06b3 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -646,8 +646,8 @@ static bool mapping_needs_writeback(struct address_space *mapping) > return mapping->nrpages; > } > > -static bool filemap_range_has_writeback(struct address_space *mapping, > - loff_t start_byte, loff_t end_byte) > +bool filemap_range_has_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; > @@ -667,34 +667,8 @@ static bool filemap_range_has_writeback(struct address_space *mapping, > } > rcu_read_unlock(); > return page != NULL; > - > -} > - > -/** > - * 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) > -{ > - if (!mapping_needs_writeback(mapping)) > - return false; > - if (!mapping_tagged(mapping, PAGECACHE_TAG_DIRTY) && > - !mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK)) > - return false; > - return filemap_range_has_writeback(mapping, start_byte, end_byte); > } > -EXPORT_SYMBOL_GPL(filemap_range_needs_writeback); > +EXPORT_SYMBOL_GPL(filemap_range_has_writeback); > > /** > * filemap_write_and_wait_range - write out & wait on a file range > > -- > Jens Axboe >
On 12/3/21 11:14 AM, Matthew Wilcox wrote: > On Fri, Dec 03, 2021 at 11:01:14AM -0700, Jens Axboe wrote: >> On 12/3/21 10:57 AM, Jens Axboe wrote: >>>> I'm happy with this, if you just move it to pagemap.h >>> >>> OK, I'll try it out. >> >> Wasn't too bad at all, actually just highlighted that I missed removing >> the previous declaration of filemap_range_needs_writeback() in fs.h >> I'll do a full compile and test, but this seems sane. >> >> commit 63c6b3846b77041d239d5b5b5a907b5c82a21c4c >> Author: Jens Axboe <axboe@kernel.dk> >> Date: Thu Oct 28 08:47:05 2021 -0600 >> >> mm: move filemap_range_needs_writeback() into header >> >> No functional changes in this patch, just in preparation for efficiently >> calling this light function from the block O_DIRECT handling. >> >> Signed-off-by: Jens Axboe <axboe@kernel.dk> > > Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org> Great, thanks for the review Willy!
diff --git a/include/linux/fs.h b/include/linux/fs.h index bbf812ce89a8..0cc4f5fd4cfe 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -2845,6 +2845,41 @@ static inline int filemap_fdatawait(struct address_space *mapping) return filemap_fdatawait_range(mapping, 0, LLONG_MAX); } +/* Returns true if writeback might be needed or already in progress. */ +static inline bool mapping_needs_writeback(struct address_space *mapping) +{ + return mapping->nrpages; +} + +bool filemap_range_has_writeback(struct address_space *mapping, + loff_t start_byte, loff_t end_byte); + +/** + * 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. + */ +static inline bool filemap_range_needs_writeback(struct address_space *mapping, + loff_t start_byte, + loff_t end_byte) +{ + if (!mapping_needs_writeback(mapping)) + return false; + if (!mapping_tagged(mapping, PAGECACHE_TAG_DIRTY) && + !mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK)) + return false; + return filemap_range_has_writeback(mapping, start_byte, end_byte); +} + extern bool filemap_range_has_page(struct address_space *, loff_t lstart, loff_t lend); extern bool filemap_range_needs_writeback(struct address_space *, diff --git a/mm/filemap.c b/mm/filemap.c index daa0e23a6ee6..65238440fa0a 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -640,14 +640,8 @@ int filemap_fdatawait_keep_errors(struct address_space *mapping) } EXPORT_SYMBOL(filemap_fdatawait_keep_errors); -/* Returns true if writeback might be needed or already in progress. */ -static bool mapping_needs_writeback(struct address_space *mapping) -{ - return mapping->nrpages; -} - -static bool filemap_range_has_writeback(struct address_space *mapping, - loff_t start_byte, loff_t end_byte) +bool filemap_range_has_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; @@ -667,34 +661,8 @@ static bool filemap_range_has_writeback(struct address_space *mapping, } rcu_read_unlock(); return page != NULL; - -} - -/** - * 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) -{ - if (!mapping_needs_writeback(mapping)) - return false; - if (!mapping_tagged(mapping, PAGECACHE_TAG_DIRTY) && - !mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK)) - return false; - return filemap_range_has_writeback(mapping, start_byte, end_byte); } -EXPORT_SYMBOL_GPL(filemap_range_needs_writeback); +EXPORT_SYMBOL_GPL(filemap_range_has_writeback); /** * filemap_write_and_wait_range - write out & wait on a file range
No functional changes in this patch, just in preparation for efficiently calling this light function from the block O_DIRECT handling. Signed-off-by: Jens Axboe <axboe@kernel.dk> --- include/linux/fs.h | 35 +++++++++++++++++++++++++++++++++++ mm/filemap.c | 38 +++----------------------------------- 2 files changed, 38 insertions(+), 35 deletions(-)