Re: [PATCH 1/2] iomap: use page dirty state to seek data over unwritten extents

[Brian Foster <bfoster@redhat.com>]

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@redhat.com>
> ---

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
>