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