On Fri, 28 Oct 2022 08:54:28 -0400 Brian Foster <bfoster@xxxxxxxxxx> 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?
