bug-coreutils
[Top][All Lists]
Advanced

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

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


From: Jim Meyering
Subject: bug#6131: [PATCH]: fiemap support for efficient sparse file copy
Date: Wed, 16 Jun 2010 08:45:08 +0200

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

> But this raises another issue.  Under this proposal, the default behavior
> copies physical _holes_, but it doesn't copy physical _extents_.
> For example, suppose the input file has a single 128 MiB extent.

Yes, I saw that already, in testing, even with relatively
simply structured inputs.  That's why the test script had to
be adjusted to accommodate differences in extent layout,
effectively merging split extents when comparing src and dest
filefrag -v output.

> The
> proposal could create an output file with two 64-MiB extents
> that are logically contiguous.  This is because the code does not advise the
> output file about the extents that the input file had, and the operating
> system might assign a smaller output extent at first, discovering only too 
> late
> that more space was needed.
>
> If the intent is to copy the physical extents, then the proposed code

That appears to be infeasible.

> needs to be fixed so that it uses fallocate() to attempt to create the
> same extents in the destination that existed in the source.  Obviously
> this could fail for any number of reasons, for example, the destination
> file system might be different from the source; but the goal would be
> to preserve extents if the underlying OS is "nice" about implementing
> fallocate().
>
> Which raises another question: should cp attempt to create _better_
> extents in the destination than in the source?  For example, if

It's already great that most of this functionality
appears to be usable on four(!) file system types.
I don't want to push our luck.

> This all is getting a bit complicated, I'm afraid.  Perhaps it'd be better
> to try this stuff out with a new option, "cp --sparse=extents" say, and
> keep the default as-is for a while?

I think we can keep it conceptually simple enough
that it will be safe to make fiemap copying the default.





reply via email to

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