GNU bug report logs - #6131
[PATCH]: fiemap support for efficient sparse file copy

Previous Next

Full log

Message #266 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "jeff.liu" <jeff.liu <at> oracle.com>
To: Joel Becker <Joel.Becker <at> oracle.com>
Cc: Sunil Mushran <sunil.mushran <at> oracle.com>, Paul Eggert <eggert <at> CS.UCLA.EDU>,
	bug-coreutils <at> gnu.org, Jim Meyering <jim <at> meyering.net>,
	Chris Mason <chris.mason <at> oracle.com>,
	Pádraig Brady <P <at> draigBrady.com>,
	Tao Ma <tao.ma <at> oracle.com>
Subject: Re: bug#6131: [PATCH]: fiemap support for efficient sparse file copy
Date: Sat, 17 Jul 2010 14:23:26 +0800

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.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.