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.
next prev parent reply other threads:[~2026-10-07 13:11 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-07 2:03 Daejun Park via B4 Relay
2026-10-07 13:11 ` Christoph Hellwig [this message]
2026-10-07 14:58 ` Chuck Lever
2026-10-07 16:51 ` Darrick J. Wong
[not found] ` <CGME20261007131141epcas2p46e1bc1592a0789298bc9f7e13c7ad393@epcms2p8>
2026-10-08 1:43 ` Daejun Park
[not found] ` <CGME20261007165105epcas2p264fc1fd07bab1fb877a6695062f48322@epcms2p7>
2026-10-08 1:47 ` Daejun Park
2026-10-08 14:14 ` (2) " Chuck Lever
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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®