Re: [PATCH] filemap: skip range writeback if end offset precedes start

[Andrew Morton <akpm@linux-foundation.org>]

On Fri, 28 Oct 2022 08:54:28 -0400 Brian Foster <bfoster@redhat.com> wrote:

> A call to file[map]_write_and_wait_range() with an end offset that
> precedes the start offset but happens to land in the same page can
> trigger writeback submission but fail to wait on the submitted page.
> Writeback submission occurs because __filemap_fdatawrite_range()
> passes both offsets down into write_cache_pages(), which rounds down
> to page indexes before it starts processing writeback.
> __filemap_fdatawait_range() immediately returns if the specified end
> offset precedes the start offset, however.
> 
> I suspect these checks are primarily intended to handle overflow
> conditions. I happened to notice this behavior when investigating an
> unrelated problem and observed that a filemap_write_and_wait_range()
> call with unexpected parameters had seemingly unpredictable latency.
> That latency turned out to be the submission path occasionally
> waiting on writeback state of the page (i.e. from
> write_cache_pages()) before issuing the currently requested
> writepage and then unconditionally failing to wait on the latter via
> __filemap_fdatawait_range().
> 
> This could probably be reasonably fixed to either elide the
> submission, as this patch does, or modify the fdatawait path to
> check the page indexes instead of the unaligned offsets. After
> poking around a bit, it seemed more consistent with various other
> filemap interfaces to check the offsets in the write path and return
> if the end offset is not >= the start. For example,
> filemap_range_has_page() and filemap_range_has_writeback() both
> include similar byte granularity checks.
> 
> ...
>
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -418,6 +418,9 @@ int __filemap_fdatawrite_range(struct address_space *mapping, loff_t start,
>  		.range_end = end,
>  	};
>  
> +	if (end < start)
> +		return 0;
> +
>  	return filemap_fdatawrite_wbc(mapping, &wbc);
>  }

Is there any way in which this condition can be triggered from
userspace?  Or from any non-buggy kernelspace?

Should we have a WARN_ON() in there to detect this?