[PATCH] xfs: make iomap_begin functions trim iomaps consistently

[PATCH] xfs: make iomap_begin functions trim iomaps consistently

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Historically, the XFS iomap_begin function only returned mappings for
exactly the range queried, i.e. it doesn't do XFS_BMAPI_ENTIRE lookups.
The current vfs iomap consumers are only set up to deal with trimmed
mappings.  xfs_xattr_iomap_begin does BMAPI_ENTIRE lookups, which is
inconsistent with the current iomap usage.  Remove the flag so that both
iomap_begin functions behave the same way.

FWIW this also fixes a behavioral regression in xattr FIEMAP that was
introduced in 4.8 wherein attr fork extents are no longer trimmed like
they used to be.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_iomap.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 33eb4fb..7ab52a8 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -1213,7 +1213,7 @@ xfs_xattr_iomap_begin(
 
 	ASSERT(ip->i_d.di_aformat != XFS_DINODE_FMT_LOCAL);
 	error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb, &imap,
-			       &nimaps, XFS_BMAPI_ENTIRE | XFS_BMAPI_ATTRFORK);
+			       &nimaps, XFS_BMAPI_ATTRFORK);
 out_unlock:
 	xfs_iunlock(ip, lockmode);
 

Re: [PATCH] xfs: make iomap_begin functions trim iomaps consistently

[Christoph Hellwig]

On Wed, Dec 06, 2017 at 10:38:07AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Historically, the XFS iomap_begin function only returned mappings for
> exactly the range queried, i.e. it doesn't do XFS_BMAPI_ENTIRE lookups.
> The current vfs iomap consumers are only set up to deal with trimmed
> mappings.  xfs_xattr_iomap_begin does BMAPI_ENTIRE lookups, which is
> inconsistent with the current iomap usage.  Remove the flag so that both
> iomap_begin functions behave the same way.
> 
> FWIW this also fixes a behavioral regression in xattr FIEMAP that was
> introduced in 4.8 wherein attr fork extents are no longer trimmed like
> they used to be.

I would still prefer to trim in the iomap code.  But I'll need to find
some time to look into that, so until then here is my reluctant ACK:

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

Re: [PATCH] xfs: make iomap_begin functions trim iomaps consistently

[Darrick J. Wong]

On Wed, Dec 06, 2017 at 04:02:34PM -0800, Christoph Hellwig wrote:
> On Wed, Dec 06, 2017 at 10:38:07AM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Historically, the XFS iomap_begin function only returned mappings for
> > exactly the range queried, i.e. it doesn't do XFS_BMAPI_ENTIRE lookups.
> > The current vfs iomap consumers are only set up to deal with trimmed
> > mappings.  xfs_xattr_iomap_begin does BMAPI_ENTIRE lookups, which is
> > inconsistent with the current iomap usage.  Remove the flag so that both
> > iomap_begin functions behave the same way.
> > 
> > FWIW this also fixes a behavioral regression in xattr FIEMAP that was
> > introduced in 4.8 wherein attr fork extents are no longer trimmed like
> > they used to be.
> 
> I would still prefer to trim in the iomap code.  But I'll need to find
> some time to look into that, so until then here is my reluctant ACK:

I started looking into that -- the iomap trimming is easy, but the data
fork iomap_begin (with a BMAPI_ENTIRE lookup) gets tripped up on
trimming the iomap around shared extents, because if we have an extent:
(R = regular, S = shared block)

01234567
RRRSSSSS

and we ask for iomap_begin(off=3, len=5) we return RRR (because we trim
to the first change in sharedness status) and not the SSSSS we expect and
then everything goes nuts.... so in the meantime we'll just restore the
old XFS behavior and bikeshed fiemap^W^Wclean it up later.

--D

> Reviewed-by: Christoph Hellwig <hch@lst.de>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html