Re: [PATCH] xfs: Make fiemap works with sparse file.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Eric Sandeen wrote:
Tao Ma wrote:
In xfs_vn_fiemap, we set bvm_count to fi_extent_max + 1 and want
to return fi_extent_max extents, but actually it won't work for
a sparse file. The reason is that in xfs_getbmap we will
calculate holes and set it in 'out', while out is malloced by
bmv_count(fi_extent_max+1) which didn't consider holes. So in the
worst case, if 'out' vector looks like
[hole, extent, hole, extent, hole, ... hole, extent, hole],
we will only return half of fi_extent_max extents.

So in xfs_vn_fiemap, we should consider this worst case. If the
user wants fi_extent_max extents, we need a 'out' with size of
2 *fi_extent_max + 1.

This all seems right to me, though your commit message above (+1)
doesn't match the comment and code in the patch (+2)
oh, yes, I will change the commit log and send a v2.
and can I add your ack for this patch?

Regards,
Tao
-Eric

Cc: Alex Elder <aelder@xxxxxxx>
Cc: Christoph Hellwig <hch@xxxxxx>
Cc: Dave Chinner <david@xxxxxxxxxxxxx>
Signed-off-by: Tao Ma <tao.ma@xxxxxxxxxx>
---
 fs/xfs/linux-2.6/xfs_iops.c |   16 ++++++++++++++--
 1 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/linux-2.6/xfs_iops.c b/fs/xfs/linux-2.6/xfs_iops.c
index 9c8019c..1db92e3 100644
--- a/fs/xfs/linux-2.6/xfs_iops.c
+++ b/fs/xfs/linux-2.6/xfs_iops.c
@@ -672,9 +672,21 @@ xfs_vn_fiemap(
 	else
 		bm.bmv_length = BTOBB(length);
- /* We add one because in getbmap world count includes the header */
+	/*
+	 * It is a bit tricky for us to calculate the bmv_count from
+	 * fi_extent_max.
+	 * If we support to return fi_extent_max extents to the user,
+	 * we need at most 2 * fi_extent_max + 1 for bmv_count since
+	 * in xfs_getbmap we will calculate holes while fi_extent_max
+	 * don't have them. So in the worst case, bmv can looks like
+	 * [hole, extent, hole, extent, hole, ... hole, extent, hole].
+	 * So there will be 2 *fi_extent_max + 1.
+	 * What's more, in getbmap world count have to include the
+	 * header, so we need another bmv. So the total number will
+	 * be 2 * fieinfo->fi_extents_max + 2.
+	 */
 	bm.bmv_count = !fieinfo->fi_extents_max ? MAXEXTNUM :
-					fieinfo->fi_extents_max + 1;
+			2 * fieinfo->fi_extents_max + 2;
 	bm.bmv_count = min_t(__s32, bm.bmv_count,
 			     (PAGE_SIZE * 16 / sizeof(struct getbmapx)));
 	bm.bmv_iflags = BMV_IF_PREALLOC;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs


[Index of Archives]     [Linux XFS Devel]     [Linux Filesystem Development]     [Filesystem Testing]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux