On Mon, Nov 27, 2017 at 09:44:34AM -0800, Darrick J. Wong wrote: > On Thu, Nov 23, 2017 at 01:40:03PM +0200, Nikolay Borisov wrote: > > For file extents xfs currently calls xfs_bmapi_read with flag set to 0, > > meaning extents are going to be truncated to the requested range. Since the > > same codepath is used for fiemap this means xfs is special in that regard, since > > other filesystems (ext4/btrfs) do not trim extents for fiemap. Make the behavior > > consistent by always passing XFS_BMAPI_ENTIRE. A full xfstest run validated that > > this doesn't regress on ordinary read/write IO. > > > > Signed-off-by: Nikolay Borisov <nborisov@xxxxxxxx> > > Looks ok, will also test... > Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx> ...and now withdrawn. Looking at iomap_apply, it seems we call ->iomap_begin, and at line 73 we trim an iomap that is longer than the range we requested: /* * Cut down the length to the one actually provided by the filesystem, * as it might not be able to give us the whole size that we requested. */ if (iomap.offset + iomap.length < pos + length) length = iomap.offset + iomap.length - pos; However, we do not trim the beginning off an iomap that starts before the 'pos' we passed to ->iomap_begin, so we're left with an iomap that can begin before the range. FWIW I ran xfstests (like I told you to on IRC) and saw regressions in a whole bunch of tests: generic/170 generic/287 generic/295 generic/326 generic/330 generic/333 generic/372 xfs/214 xfs/237 xfs/249 xfs/258 xfs/346 xfs/420 xfs/421 >From a brief glance it looks as though most of the iomap_apply'ers try to trim the iomap before using it, but clearly there's something wrong here. --D > > > --- > > 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 f179bdf1644d..8942324a4d3d 100644 > > --- a/fs/xfs/xfs_iomap.c > > +++ b/fs/xfs/xfs_iomap.c > > @@ -1008,7 +1008,7 @@ xfs_file_iomap_begin( > > end_fsb = XFS_B_TO_FSB(mp, offset + length); > > > > error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb, &imap, > > - &nimaps, 0); > > + &nimaps, XFS_BMAPI_ENTIRE); > > if (error) > > goto out_unlock; > > > > -- > > 2.7.4 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > > the body of a message to majordomo@xxxxxxxxxxxxxxx > > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
