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

[jeff.liu]

Pádraig Brady wrote:
> On 16/06/10 09:49, Jim Meyering wrote:
>> Pádraig Brady wrote:
>>
>>> On 16/06/10 07:45, Jim Meyering wrote:
>>>> Paul Eggert wrote:
>>>>> On 06/15/2010 02:43 PM, Jim Meyering wrote:
>>>>>> I think that copying physical holes via FIEMAP should be the default, 
>>>>>> when
>>>>>> possible.  One problem is that the current code on the fiemap-copy branch
>>>>>> does not honor --sparse=WHEN when in fiemap-copying mode.  The solution
>>>>>> would seem to be to change the regular-file-copying loop in the 
>>>>>> fiemap_copy
>>>>>> function to use the same hole-preserving code that is used in copy_reg.
>>>>> I assume that the solution would be used only with cp --sparse=always?
>>>>> (Otherwise, it would amount to copying logical holes by default.)
>>>> Right.
>>>> I.e., use the same code that honors (or not) the "make_holes" variable.
>>>>
>>>>> If so, under this proposal, --sparse=always would copy logical holes,
>>>>> --sparse=never would never copy holes, and --sparse=auto (the default)
>>>>> would copy physical holes if supported, else it would copy logical
>>>>> holes if the file seems to have enough physical holes, and otherwise
>>>>> it would copy no holes.  (Whew!)
>>>> Right.  With --sparse=always and fiemap support, it would take advantage
>>>> of existing extents to minimize copying time, and for the nontrivial
>>>> extents, it would detect/induce new holes when possible.
>>>>
>>>> Perhaps we need a new logical option that would make a difference
>>>> only when there are nontrivial fiemap extents:
>>>>   - read nontrivial extents, as is done now
>>>>   - read them, *and* search-for/induce-holes as we do in legacy
>>>>       copy for --sparse=always.
>>>>
>>>> Or, putting it another way, perhaps we need a new command line
>>>> option to control whether we even attempt a fiemap copy.
>>>> IMHO the default should be to enable it, once all of the
>>>> underlying bits are deemed to be stable enough.
>>>>
>>>>     --fiemap
>>>>     --no-fiemap
>>>>
>>>> Then, --fiemap --sparse=never would do what the existing fiemap_copy
>>>> function does, and --fiemap --sparse=always would work once the
>>>> internal-to-fiemap_copy copying code is adjusted to use the
>>>> hole-preserving code in copy_reg.
>>>>
>>>> Then, to guarantee the legacy --sparse=never behavior,
>>>> one would have to specify --no-fiemap --sparse=never
>>> I would suggest not to add options like this,
>>> and only do what's possible without new options
>>> as it would push too many implementation details
>>> to the docs/users IMHO.
>> Good point.  Smaller/simpler would be better.
>> It'd be great if we can eliminate as never-useful one of the
>> new combinations.
>>
>> Currently on the fiemap-copy branch, once we get a single
>> successful ioctl, the code will not bother with the usual
>> hole-detecting/introducing technique implied by --sparse=always.
>>
>> Should it do that ever?  Always?
>> Does this need an option?


Hello All,

I am very sorry for the late response! I have an urgent task need to deliver in 
the past few weeks.

Thanks for all your suggestions, I would like to improve the fiemap-copy 
accordingly, so does the
following sentence is the final decision?

> I don't see the need for new options TBH
> I see fiemap just as a way to efficiently detect/read holes,
> and should have no bearing on the destination.
> 
> cp --sparse=auto (this is currently what cp does by default)
>   recreate the original fiemap holes or resort to existing
>   heuristic if fiemap not available
> cp --sparse=never
>   write all data, but use fiemap if available to efficiently read
> cp --sparse=always
>   recreate original holes and perhaps extend add to them for
>   other runs of zero bytes. Without having looked at the code
>   I see this as a little tricky to mix with fiemap.
>   Now since fiemap is only an optimization we can skip it
>   completely for this uncommon case if too tricky (just add a FIXME for now).

Current code handle 'cp --sparse=auto' and 'cp --sparse=always' in same way 
since these two options
all setup 'make_holes' to True, though '--sparse=auto' use a heuristic to 
determine whether SRC_NAME
contains any sparse blocks.

For 'cp --sparse=never', when detected holes from SRC file, do not lseek(2) 
against DST file,
instead, write ZEROs to DST file, Am I right?

If so, IMHO, we can pass 'sparse_mode' to fiemap_copy(), then decide how to do 
operation against DST
file according to its mode.

In addition to this, I'd like to fix another issue in current code as Paul 
suggested,
1. Do not allocate buffer in fiemap_copy() for data pass between SRC and DST 
files, instead, take
advantage of the buffer allocated outside.

2. Performance optimization, invoke fallocate(2) if an extent flag is UNWRITTEN
For this case, maybe we have to wait until fallocate interface become stable 
just as Pádraig
methoned before:
http://lists.gnu.org/archive/html/bug-coreutils/2009-05/msg00260.html

> cheers,
> Pádraig.

Thanks,
-Jeff


-- 
With Windows 7, Microsoft is asserting legal control over your computer and is 
using this power to
abuse computer users.