Messages in this thread | | | Date | Sat, 12 Jun 2010 07:37:31 +0800 | From | Tao Ma <> | Subject | Re: [PATCH] xfs: Make fiemap works with sparse file. |
| |
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@sgi.com> >> Cc: Christoph Hellwig <hch@lst.de> >> Cc: Dave Chinner <david@fromorbit.com> >> Signed-off-by: Tao Ma <tao.ma@oracle.com> >> --- >> 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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ >
| |