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

Previous Next

Full log

View this message in rfc822 format

From: Joel Becker <Joel.Becker <at> oracle.com>
To: "jeff.liu" <jeff.liu <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: bug#6131: [PATCH]: fiemap support for efficient sparse file copy
Date: Fri, 16 Jul 2010 23:14:27 -0700

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++];
}

Joel

-- 

Life's Little Instruction Book #497

	"Go down swinging."

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker <at> oracle.com
Phone: (650) 506-8127

Previous Next

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