On Mon, Oct 12, 2020 at 10:03:49AM -0400, Brian Foster wrote:
> iomap seek hole/data currently uses page Uptodate state to track
> data over unwritten extents. This is odd and unpredictable in that
> the existence of clean pages changes behavior. For example:
>
> $ xfs_io -fc "falloc 0 32k" -c "seek -d 0" \
> -c "pread 16k 4k" -c "seek -d 0" /mnt/file
> Whence Result
> DATA EOF
> ...
> Whence Result
> DATA 16384
>
> Instead, use page dirty state to locate data over unwritten extents.
> This causes seek data to land on the first uptodate block of a dirty
> page since we don't have per-block dirty state in iomap. This is
> consistent with writeback, however, which converts all uptodate
> blocks of a dirty page for similar reasons.
>
> Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
> ---
JFYI that I hit a generic/285 failure with this patch. I suspect this
needs to check for Dirty || Writeback, otherwise if we see the latter
the range is incorrectly treated as a hole.
Brian
> fs/iomap/seek.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/fs/iomap/seek.c b/fs/iomap/seek.c
> index 107ee80c3568..981a74c8d60f 100644
> --- a/fs/iomap/seek.c
> +++ b/fs/iomap/seek.c
> @@ -40,7 +40,7 @@ page_seek_hole_data(struct inode *inode, struct page *page, loff_t *lastoff,
> * Just check the page unless we can and should check block ranges:
> */
> if (bsize == PAGE_SIZE || !ops->is_partially_uptodate)
> - return PageUptodate(page) == seek_data;
> + return PageDirty(page) == seek_data;
>
> lock_page(page);
> if (unlikely(page->mapping != inode->i_mapping))
> @@ -49,7 +49,8 @@ page_seek_hole_data(struct inode *inode, struct page *page, loff_t *lastoff,
> for (off = 0; off < PAGE_SIZE; off += bsize) {
> if (offset_in_page(*lastoff) >= off + bsize)
> continue;
> - if (ops->is_partially_uptodate(page, off, bsize) == seek_data) {
> + if ((ops->is_partially_uptodate(page, off, bsize) &&
> + PageDirty(page)) == seek_data) {
> unlock_page(page);
> return true;
> }
> --
> 2.25.4
>
