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.
