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@xxxxxxxxx>
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@xxxxxxxxx>
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
