Re: [PATCH 1/1] xfs: fix overlapping extents returned for pNFS LAYOUTGET

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, May 19, 2026 at 06:44:18AM -0700, Dai Ngo wrote:
> 
> On 5/18/26 11:30 PM, Christoph Hellwig wrote:
> > On Mon, May 18, 2026 at 12:55:17PM -0700, Dai Ngo wrote:
> > > As shown, the file map changes. Entry# 7 and 8 before the NFSD calls
> > > merged into entry#7 after the calls. So there must be some activities
> > > that cause the map to change. I don't know whether the activities were
> > > triggered by NFS or something in XFS or the block device layer.
> > Hmm.  We only insert layouts (and search for conflicts) after calling
> > ->proc_layoutget.  So this might be racy against unwritten extent
> > conversion, or other writers, which is a bit of an issue.  I guess
> > we need to fix nfsd4_layoutget to insert an in-progress layout before
> > calling into ->layouget.
> 
> I can't quite see the race condition regarding layoutget here. Could you
> please explain?

/me has no idea about nfs layouts but has thoughts anyway :)

1) xfs_fs_map_blocks only takes i_rwsem and XFS_ILOCK; it doesn't take
the mmap invalidation lock.  Does that mean that pagefaults could wander
in and mess with the layout?

(I think the race that Christoph is referring to here is that any other
thread can wander in and change the mapping after a ->map_blocks call
returns, because nfs isn't holding any kind of lock on the inode)

2) Now that NFS apparently can serve up multiple mappings at once,
should ->map_blocks pass in an array element count so that we can do
multiple iomaps in a single lock cycle?

3) Do the reflink and realtime inode checks need to be re-assessed after
grabbing the ilock since they can change?

--D

> > > However, based on this data I think it's better to change the bmapi_flags
> > > from XFS_BMAPI_ENTIRE to '0' to address the overlap issue.
> > We absolutely should be doing that as I said from my first reply.
> > Still trying to understand what is going on here at a higher level,
> > though.
> 
> I'll resubmit the patch series with both fixes combined: the uninitialized
> imap handling in the error path and the replacement of XFS_BMAPI_ENTIRE
> with 0.
> 
> Thanks,
> -Dai
> 
>