[Top][All Lists]

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

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

From: jeff.liu
Subject: bug#6131: [PATCH]: fiemap support for efficient sparse file copy
Date: Sat, 17 Jul 2010 14:23:26 +0800
User-agent: Thunderbird (X11/20080505)

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

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.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]