All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: daejun7.park@samsung.com
Cc: Chuck Lever <cel@kernel.org>, Jeff Layton <jlayton@kernel.org>,
	NeilBrown <neil@brown.name>,
	Olga Kornievskaia <okorniev@redhat.com>,
	Dai Ngo <Dai.Ngo@oracle.com>, Tom Talpey <tom@talpey.com>,
	Christoph Hellwig <hch@lst.de>,
	"Darrick J. Wong" <djwong@kernel.org>,
	Sergey Bashirov <sergeybashirov@gmail.com>,
	Carlos Maiolino <cem@kernel.org>,
	Amir Goldstein <amir73il@gmail.com>,
	linux-nfs@vger.kernel.org, linux-xfs@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] nfsd: do not return overlapping extents in a block layout
Date: Wed, 7 Oct 2026 15:11:33 +0200	[thread overview]
Message-ID: <20261007131133.GB30647@lst.de> (raw)
In-Reply-To: <20261007-nfsd-block-trim-v1-1-53370f97047a@samsung.com>

On Wed, Oct 07, 2026 at 11:03:01AM +0900, Daejun Park via B4 Relay wrote:
> From: Daejun Park <daejun7.park@samsung.com>
> 
> Since commit cc6c40e09d7b ("NFSD/blocklayout: Support multiple extents
> per LAYOUTGET"), nfsd4_block_proc_layoutget() calls ->map_blocks once
> per extent of a LAYOUTGET, each time for the range left after the
> previous extent. nfsd4_block_map_extent() takes a mapping that starts at
> the offset asked for or below it, but only the first extent may start
> below it. A later extent that starts below its offset overlaps the
> extents before it, which RFC 5663 section 2.3.1 does not allow. The
> Linux client rejects such a layout (verify_extent() returns -EIO), and
> the I/O that needed it fails. The block and SCSI layouts share this
> code.
> 
> XFS, the only ->map_blocks implementation, used to map the whole extent
> that contains the offset. Together with one call per extent, that gave
> overlapping layouts in v6.19 and v7.0: allocating the blocks for one
> extent of a write layout could merge the extent that the next call
> starts in with the unwritten extents before it. Since commit
> 36ca6f11424a ("xfs: fix overlapping extents returned for pNFS
> LAYOUTGET"), XFS does not map below the offset, so this patch changes
> nothing with current XFS. It only stops nfsd from relying on that: trim
> each extent after the first so that it starts at the offset asked for,
> and move its volume offset by the same amount unless it is a NONE_DATA
> extent, which has none.
> 
> nfsd leaves the end of a mapping alone, as only the filesystem knows
> where it should end. It does need a mapping that contains the offset
> asked for, which RFC 5663 section 2.3.1 also requires of the first
> extent. A mapping that does not would leave a gap in the layout or make
> the length computed from it wrap around, so warn and return
> NFS4ERR_LAYOUTUNAVAILABLE; the client then does the I/O through the
> metadata server. iomap_iter_done() has the same check, as a
> WARN_ON_ONCE(), for ->iomap_begin(), and nfsd4_block_map_extent()
> already warns and fails this way for a mapping of an unexpected type.
> Document in exportfs_block.h that the mapping must contain the offset.
> 
> On a test kernel whose xfs_fs_map_blocks() maps with XFS_BMAPI_ENTIRE
> and does not trim, as XFS did before that commit, a write layout over a
> hole between two unwritten blocks comes back as 0+4096, 4096+4096 and
> 0+12288, and the pynfs test BLOCK5 fails in five runs out of five.
> fstests generic/075, 091 and 263 over the block layout fail with an EIO
> or a zero-length O_DIRECT write, and bl_alloc_lseg() on the client
> returns -EIO three times. With this patch on top, the third extent is
> 8192+4096, BLOCK5 passes in five runs out of five, generic/091 and 263
> pass, generic/075 fails with the fsx "Size error" that it also fails
> with on nfsd-testing, and bl_alloc_lseg() returns no error.

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

But given that you're pretty active in this part of nfsd now, can you
also look into my earlier suggestion to allow a single map_blocks
return multiple mappings?  That solves more of the root cause of
creating incoherencies.


  reply	other threads:[~2026-10-07 13:11 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-07  2:03 [PATCH] nfsd: do not return overlapping extents in a block layout Daejun Park via B4 Relay
2026-10-07  2:03 ` Daejun Park
2026-10-07 13:11 ` Christoph Hellwig [this message]
2026-10-08  1:43   ` Daejun Park
2026-10-07 14:58 ` Chuck Lever
2026-10-07 16:51 ` Darrick J. Wong
2026-10-08  1:47   ` Daejun Park
2026-10-08 14:14     ` (2) " Chuck Lever
2026-10-12  2:52       ` Daejun Park

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=20261007131133.GB30647@lst.de \
    --to=hch@lst.de \
    --cc=Dai.Ngo@oracle.com \
    --cc=amir73il@gmail.com \
    --cc=cel@kernel.org \
    --cc=cem@kernel.org \
    --cc=daejun7.park@samsung.com \
    --cc=djwong@kernel.org \
    --cc=jlayton@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=neil@brown.name \
    --cc=okorniev@redhat.com \
    --cc=sergeybashirov@gmail.com \
    --cc=tom@talpey.com \
    /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.