Re: [PATCH] copy: adjust fiemap handling of sparse files

[Jim Meyering]

Pádraig Brady wrote:
> On 17/03/11 23:00, Pádraig Brady wrote:
>> On 17/03/11 19:29, Mike Frysinger wrote:
>>> On Thursday, March 17, 2011 05:32:33 Pádraig Brady wrote:
>>>> This is the kind of patch appropriate for a distro,
>>>> as they may or may not have a fix backported to their kernel.
>>>
>>> Gentoo, being a source distribution, has no kernel requirement.  people are
>>> free to run on pretty much anything.  we dont have the luxury of saying
>>> "upgrade your kernel package and reboot".  especially since we also support
>>> running Gentoo inside of other distros (often times as non-root user) where
>>> people dont have the ability at all to upgrade.
>>>
>>>> I presume you're referring to coreutils bug 8001.
>>>> I didn't realise ext4 was also affected.
>>>> Is this specific to the compress flag?
>>>> What was the specific fix to btrfs &/or ext4 in 2.6.38?
>>>
>>> this is the ext4-specific issue i'm avoiding:
>>> http://www.spinics.net/lists/linux-ext4/msg23430.html
>>
>> Ah that issue. That's what the syncing required in this test
>> was working around:
>> http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=blob;f=tests/cp/sparse-fiemap;h=a2460a08;hb=HEAD
>> Note Fedora 15 about to be released is using the new fiemap code in cp,
>> but is also based on 2.6.38 and so will have the fix soon,
>> if it does not already have it.
>
> I also now remember this discussion:
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=6131
> where FIEMAP_FLAG_SYNC was introduced in cp,
> I think to work around this same bug in BTRFS and EXT4.
> This flag in ineffective though on ext4 loopback
> and thus I needed to add the syncs to the test as ref'd above.
>
>>>> Also note the make_holes heuristic variable is no
>>>> longer used in extent_copy due to this patch which
>>>> came after the 8.10
>>>> http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=275c831a
>>>
>>> i'll worry about it once 8.11 is released ;)
>>> -mike
>>
>> You might be safer to just bypass extent_copy for kernels < 2.6.38
>> I'm 30:70 for doing that in upstream coreutils
>
> So am I right in thinking FIEMAP_FLAG_SYNC is no longer needed with 2.6.38.
> I'm quite worried about implicit syncing in cp actually, where it might
> introduce bad performance regressions.

Good point.

> Maybe we could disable this flag if XFS || kernel >= 2.6.38?
> Or maybe we might do as you suggest above and disable extent_copy()
> completely, for kernels before 2.6.38.
> Not an ideal situation at all :(

If there really is risk of data loss with ext4 on kernels < 2.6.38,
even if it's only on unusual files, then we'll have to consider
using a kernel version check to disable extent_copy.

Is there a stand-alone ext4 reproducer?