bug#6131: [PATCH]: fiemap support for efficient sparse file copy

[jeff.liu]

Joel Becker wrote:
> On Sat, Jul 17, 2010 at 10:11:30AM +0800, jeff.liu wrote:
>> Joel Becker wrote:
>>> On Fri, Jul 16, 2010 at 08:53:27AM -0700, Paul Eggert wrote:
>>>> I haven't had time to look at it carefully, but here's a very brief
>>>> review.  The code you sent, like what's in the fiemap branch, has
>>>> a separate version of a chunk of copy.c that does both reading
>>>> and writing and optimizes both reading and writing by invoking the fiemap 
>>>> ioctls
>>>> at strategic locations.  Instead, it would be better to have
>>>> a module that separates out the efficient-read stuff by telling
>>>> copy.c where the next significant input extent is, and then modify copy.c
>>>> to use that module.  On hosts that do not support fiemap, the module
>>>> would simply report the entire input file as that file's only extent.
>>>     Precisely.  The sparse-core.c or whatever it is called shouldn't
>>> be doing the copy, it should just provide:
>>>
>>> handle = init_extent_scan(fd);
>>> while (get_next_extent(handle, &extent_start, &extent_len)) {
>>>     ...
>>> }
>>> close_extent_scan(handle);
>>>
>>>     Then copy.c just implements this loop and the '...' part.
>>>
>>> Joel
>>>
>> yes, its better to separate copy and extent scan, and its not difficult to 
>> implement.  But I was
>> wondering to return an array of extents info or just return one extent info 
>> for each scan?
> 
>       get_next_extent() just returns one extent, but the caller has no
> idea what is hanging off of handle.  In fiemap, it could be a large
> array of extents you've cached during init_extent_scan().  For Solaris
> it might be some placeholder.
> 
>> I would like to work out an unique interface could work for both Linux and 
>> Solaris, for Solaris
>> SEEK_DATA/HOLES stuff, looks its convinent to just return next extent info 
>> every time.
>>
>> But for fiemap, maybe its better to return an extents_info_array as currentt 
>> design to reduce the
>> ioctl(2)  calls.
> 
>       You don't need multiple ioctl(2) calls.  Here's a trivial
> example:
> 
> void *init_extent_scan(int fd)
> {
>     struct fiemap *handle;
> 
>     handle = malloc(sizeof(struct fiemap) +
>                     (EXTENTS_PER_IOCTL * sizeof(struct fiemap_extent)));
>     handle->fm_extent_count = EXTENTS_PER_IOCTL;
>     if (!ioctl(fd, FS_IOC_FIEMAP, handle))
>         return handle;
> 
>     if (lseek(fd, SEEK_HOLE) >= 0)
>         return (void *)-1;
> 
>     return NULL;
> }
> 
> loff_t get_next_extent(void *handle, loff_t *start, loff_t *len)
> {
>     if (handle == (void *)-1)
>         /* Do SEEK_HOLE thing */
>     else if (handle)
>         return handle->fm_extents[next_one++];
> }
Thanks Joel, I understood your idea now!

> 
> Joel
> 
Regards,
-Jeff

-- 
The knowledge you get, no matter how much it is, must be possessed yourself and 
nourished with your
own painstaking efforts and be your achievement through hard work.