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

[Pádraig Brady]

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.
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 :(

cheers,
Pádraig.