All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: Brian Foster <bfoster@redhat.com>
Cc: linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org
Subject: Re: [PATCH v3 1/8] xfs: fix iomap hole map reporting for zoned zero range
Date: Mon, 9 Mar 2026 10:11:00 -0700	[thread overview]
Message-ID: <20260309171100.GL6033@frogsfrogsfrogs> (raw)
In-Reply-To: <20260309134506.167663-2-bfoster@redhat.com>

On Mon, Mar 09, 2026 at 09:44:59AM -0400, Brian Foster wrote:
> The hole mapping logic for zero range in zoned mode is not quite
> correct. It currently reports a hole whenever one exists in the data
> fork. If the first write to a sparse range has completed and not yet
> written back, the blocks exist in the COW fork as delalloc until
> writeback completes, at which point they are allocated and mapped
> into the data fork. If a zero range occurs on a range that has not
> yet populated the data fork, we will incorrectly report it as a
> hole.
> 
> Note that this currently functions correctly because we are bailed
> out by the pagecache flush in iomap_zero_range(). If a hole or
> unwritten mapping is reported with dirty pagecache, it assumes there
> is pending data, flushes to induce any pending block
> allocations/remaps, and retries the lookup. We want to remove this
> hack from iomap, however, so update iomap_begin() to only report a
> hole for zeroing when one exists in both forks.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>
> ---
>  fs/xfs/xfs_iomap.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index be86d43044df..8c3469d2c73e 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -1651,14 +1651,6 @@ xfs_zoned_buffered_write_iomap_begin(
>  				&smap))
>  			smap.br_startoff = end_fsb; /* fake hole until EOF */
>  		if (smap.br_startoff > offset_fsb) {
> -			/*
> -			 * We never need to allocate blocks for zeroing a hole.
> -			 */
> -			if (flags & IOMAP_ZERO) {
> -				xfs_hole_to_iomap(ip, iomap, offset_fsb,
> -						smap.br_startoff);
> -				goto out_unlock;
> -			}
>  			end_fsb = min(end_fsb, smap.br_startoff);
>  		} else {
>  			end_fsb = min(end_fsb,
> @@ -1690,6 +1682,16 @@ xfs_zoned_buffered_write_iomap_begin(
>  	count_fsb = min3(end_fsb - offset_fsb, XFS_MAX_BMBT_EXTLEN,
>  			 XFS_B_TO_FSB(mp, 1024 * PAGE_SIZE));
>  
> +	/*
> +	 * When zeroing, don't allocate blocks for holes as they are already
> +	 * zeroes, but we need to ensure that no extents exist in both the data
> +	 * and COW fork to ensure this really is a hole.

But where is the cow fork check?  iomap_iter initializes srcmap to
IOMAP_HOLE, so if we end up here on an IOMAP_ZERO then we've scanned the
data fork and found no mapping.  But then we jump to out_unlock, which
means we don't actually look at ip->cowfp.

<confused>

--D

> +	 */
> +	if ((flags & IOMAP_ZERO) && srcmap->type == IOMAP_HOLE) {
> +		xfs_hole_to_iomap(ip, iomap, offset_fsb, end_fsb);
> +		goto out_unlock;
> +	}
> +
>  	/*
>  	 * The block reservation is supposed to cover all blocks that the
>  	 * operation could possible write, but there is a nasty corner case
> -- 
> 2.52.0
> 
> 

  reply	other threads:[~2026-03-09 17:11 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-09 13:44 [PATCH v3 0/8] iomap, xfs: improve zero range flushing and lookup Brian Foster
2026-03-09 13:44 ` [PATCH v3 1/8] xfs: fix iomap hole map reporting for zoned zero range Brian Foster
2026-03-09 17:11   ` Darrick J. Wong [this message]
2026-03-09 18:18     ` Brian Foster
2026-03-10 14:47       ` Darrick J. Wong
2026-03-10  6:45   ` Christoph Hellwig
2026-03-09 13:45 ` [PATCH v3 2/8] xfs: flush dirty pagecache over hole in zoned mode " Brian Foster
2026-03-09 17:22   ` Darrick J. Wong
2026-03-09 18:19     ` Brian Foster
2026-03-10  6:47     ` Christoph Hellwig
2026-03-10 14:48       ` Darrick J. Wong
2026-03-10  6:45   ` Christoph Hellwig
2026-03-09 13:45 ` [PATCH v3 3/8] iomap, xfs: lift zero range hole mapping flush into xfs Brian Foster
2026-03-09 17:40   ` Darrick J. Wong
2026-03-10  6:47   ` Christoph Hellwig
2026-03-09 13:45 ` [PATCH v3 4/8] xfs: flush eof folio before insert range size update Brian Foster
2026-03-09 17:32   ` Darrick J. Wong
2026-03-09 18:24     ` Brian Foster
2026-03-09 13:45 ` [PATCH v3 5/8] xfs: look up cow fork extent earlier for buffered iomap_begin Brian Foster
2026-03-09 13:45 ` [PATCH v3 6/8] xfs: only flush when COW fork blocks overlap data fork holes Brian Foster
2026-03-09 17:47   ` Darrick J. Wong
2026-03-09 13:45 ` [PATCH v3 7/8] xfs: replace zero range flush with folio batch Brian Foster
2026-03-09 17:48   ` Darrick J. Wong
2026-03-09 13:45 ` [PATCH v3 8/8] xfs: report cow mappings with dirty pagecache for iomap zero range Brian Foster
2026-03-09 17:56   ` Darrick J. Wong
2026-03-09 18:31     ` Brian Foster
2026-03-09 18:38       ` Darrick J. Wong
2026-03-10  6:50     ` Christoph Hellwig
2026-03-10 14:52       ` Darrick J. Wong
2026-03-10 14:59         ` Christoph Hellwig
2026-03-10  6:49   ` Christoph Hellwig
2026-03-10  6:45 ` [PATCH v3 0/8] iomap, xfs: improve zero range flushing and lookup Christoph Hellwig

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=20260309171100.GL6033@frogsfrogsfrogs \
    --to=djwong@kernel.org \
    --cc=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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.