Re: [PATCH 1/2] mm: move filemap_range_needs_writeback() into header
From: Jens Axboe <axboe@kernel.dk>
Date: 2021-12-03 16:38:27
Also in:
linux-block
Subsystem:
filesystems (vfs and infrastructure), memory management, page cache, the rest · Maintainers:
Alexander Viro, Christian Brauner, Andrew Morton, Matthew Wilcox, Jan Kara, Linus Torvalds
On 12/3/21 9:35 AM, Jens Axboe wrote:
On 12/3/21 9:31 AM, Jens Axboe wrote:quoted
On 12/3/21 9:24 AM, Jens Axboe wrote:quoted
On 12/3/21 9:16 AM, Matthew Wilcox wrote:quoted
On Fri, Dec 03, 2021 at 08:38:28AM -0700, Jens Axboe wrote:quoted
+++ b/include/linux/fs.hfs.h is the wrong place for these functions; they're pagecache functionality, so they should be in pagemap.h.quoted
+/* 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 ...quoted
+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 [off-list ref]
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 [off-list ref]
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