Re: [PATCH 1/2] mm: move filemap_range_needs_writeback() into header

On 12/3/21 9:31 AM, Jens Axboe wrote:

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.h

fs.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.

-- 
Jens Axboe