From: Brian Foster <bfoster@redhat.com>
To: linux-fsdevel@vger.kernel.org
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 1/2] iomap: use page dirty state to seek data over unwritten extents
Date: Tue, 13 Oct 2020 08:30:46 -0400 [thread overview]
Message-ID: <20201013123046.GD966478@bfoster> (raw)
In-Reply-To: <20201012140350.950064-2-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
>
next prev parent reply other threads:[~2020-10-13 12:31 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-12 14:03 [PATCH 0/2] iomap: zero dirty pages over unwritten extents Brian Foster
2020-10-12 14:03 ` [PATCH 1/2] iomap: use page dirty state to seek data " Brian Foster
2020-10-13 12:30 ` Brian Foster [this message]
2020-10-13 22:53 ` Dave Chinner
2020-10-14 12:59 ` Brian Foster
2020-10-14 22:37 ` Dave Chinner
2020-10-15 9:47 ` Christoph Hellwig
2020-10-19 16:55 ` Brian Foster
2020-10-27 18:07 ` Christoph Hellwig
2020-10-28 11:31 ` Brian Foster
2020-10-12 14:03 ` [PATCH 2/2] iomap: zero cached pages over unwritten extents on zero range Brian Foster
2020-10-15 9:49 ` Christoph Hellwig
2020-10-19 16:55 ` Brian Foster
2020-10-19 18:01 ` Brian Foster
2020-10-20 16:21 ` Brian Foster
2020-10-27 18:15 ` Christoph Hellwig
2020-10-28 11:31 ` Brian Foster
2020-10-23 1:02 ` [iomap] 11b5156248: xfstests.xfs.310.fail kernel test robot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201013123046.GD966478@bfoster \
--to=bfoster@redhat.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).