[Top] [All Lists]

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

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH v2] xfs: Make fiemap works with sparse file.
From: Tao Ma <tao.ma@xxxxxxxxxx>
Date: Fri, 18 Jun 2010 10:27:49 +0800
Cc: xfs@xxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, Alex Elder <aelder@xxxxxxx>, Christoph Hellwig <hch@xxxxxx>, Eric Sandeen <sandeen@xxxxxxxxxx>, "tao.ma" <tao.ma@xxxxxxxxxx>
In-reply-to: <20100618004708.GZ6590@dastard>
References: <20100614122912.GD6590@dastard> <1276764799-4837-1-git-send-email-tao.ma@xxxxxxxxxx> <20100618004708.GZ6590@dastard>
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/20100317 Thunderbird/3.0.4
Hi Dave,

On 06/18/2010 08:47 AM, Dave Chinner wrote:
On Thu, Jun 17, 2010 at 04:53:19PM +0800, Tao Ma wrote:
Hi Dave,
On 06/14/2010 08:29 PM, Dave Chinner wrote:
I just had a thought - if you want to avoid holes being reported to
fiemap, then add a BMV_IF_NO_HOLES flag to xfs_getbmap() and skip
holes in the mappin gloop when this flag is set. That will make
fiemap fill in the full number of extents without hacking the
extent count...
Here is the updated one. I have used BVM_IF_NO_HOLES in xfs_getbmap
to skip increasing index 'cur_ext'. It is a bit ugly, see my commit
log. I guess maybe we can add another flag in xfs_bmapi so that it
don't even give us the holes?

No need...
I am fine with it.


 From cee1765ffd3e2b003b837666b4620b5107ed9ddd Mon Sep 17 00:00:00 2001
From: Tao Ma<tao.ma@xxxxxxxxxx>
Date: Thu, 17 Jun 2010 16:14:22 +0800
Subject: [PATCH v3] xfs: Make fiemap works with sparse file.

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.

This patch add a new parameter BMV_IF_NO_HOLES for bvm_iflags.
So with this flags, we don't use our 'out' in xfs_getbmap for
a hole. The solution is a bit ugly by just don't increasing
index of 'out' vector. I felt that it is not easy to skip it
at the very beginning since we have the complicated check and
some function like xfs_getbmapx_fix_eof_hole to adjust 'out'.

... because I think we can safely skip xfs_getbmapx_fix_eof_hole()
it only modifies the hole. Hence  just adding a check after the
attribute fork end check (which needs to detect a hole to terminate)
should be fine: e.g something like:

                        if (map[i].br_startblock == HOLESTARTBLOCK&&
                        whichfork == XFS_ATTR_FORK) {
                                /* came to the end of attribute fork */
                                out[cur_ext].bmv_oflags |= BMV_OF_LAST;
                                goto out_free_map;
+                       if (map[i].br_startblock == HOLESTARTBLOCK&&
+                                (iflags&  BMV_IF_NO_HOLES)) {
+                               memset(&out[cur_ext], 0, sizeof(out[cur_ext]));
+                               continue;
+                       }

Should work and avoid the worst of the ugliness.
I am afraid it doesn't work, at least from my test. It enters a dead loop.
I think the root cause is that your change doesn't update bmv_offset and bmv_length for a hole. So in the large loop,
do {
                nmap = (nexleft > subnex) ? subnex : nexleft;
                error = xfs_bmapi(NULL, ip, XFS_BB_TO_FSBT(mp, bmv->bmv_offset),
                                  XFS_BB_TO_FSB(mp, bmv->bmv_length),
                                  bmapi_flags, NULL, 0, map, &nmap,
                                  NULL, NULL);
                if (error)
                        goto out_free_map;
} while (nmap && nexleft && bmv->bmv_length);

We will dead loop there and we need xfs_getbmapx_fix_eof_hole to go out directly.


<Prev in Thread] Current Thread [Next in Thread>