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

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

On Wed, May 20, 2026 at 08:09:29AM -0700, Dai Ngo wrote:
> 
> On 5/20/26 1:24 AM, Christoph Hellwig wrote:
> > [adding Sergey, who wrote the code to allow multiple mappings]
> > 
> > On Tue, May 19, 2026 at 07:59:49AM -0700, Darrick J. Wong wrote:
> > > 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?
> > Looks like it.
> 
> I already described the scenario in which the file mapping can change
> between successive calls from nfsd—triggered by a single LAYOUTGET
> request—to xfs_fs_map_blocks().
> 
> I also don't see how page faults are relevant in the context of pNFS
> layouts. The server is not servicing actual data accesses through page
> faults when constructing a LAYOUTGET response

Some other process on the nfs server mmaps the file and generates
faults.

> > 
> > > 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?
> > I guess we need to do that, or revert the code to provide multiple maps
> > for now.
> 
> I think we should address the immediate problem first by replacing XFS_BMAPI_ENTIRE
> with 0, and handle support for returning multiple map entries from a single
> call to xfs_fs_map_blocks() in follow-up patches. That work is more involved,
> as it requires coordinated changes in both the nfsd and XFS code.

Agreed.

> > 
> > > 3) Do the reflink and realtime inode checks need to be re-assessed after
> > > grabbing the ilock since they can change?
> > Yes.
> 
> I can move the check for reflink and realtime inode inside the ilock in v2
> of this patch series.

Yeah, that probably needs to be made too. :)

--D

> -Dai
> 
>