bug#51433: cp 9.0 sometimes fails with SEEK_DATA/SEEK_HOLE

[Pádraig Brady]

On 28/10/2021 20:11, Paul Eggert wrote:
> On 10/28/21 06:54, Pádraig Brady wrote:
>
> > Further debugging from Nix folks suggest ZFS was in consideration always,
> > as invalid artifacts were written to a central cache from ZFS backed hosts.
> > So we should at least change the comment in the patch to only mention ZFS.
>
> Yes, that sounds reasonable.
>
> This ZFS bug sounds pretty serious, though. Apparently it affects star
> and other programs too. I'm not sure we should attempt to work around it
> in coreutils, if the workarounds penalize everybody not using ZFS.
>
> Is it cheap to check whether a file is actually in a ZFS filesystem?
> (Don't know how this'd work with loopback mounts, NFS, etc.) If so, it
> might be better to simply fdatasync (or even fsync) every input file
> that's on ZFS, until we know the ZFS bugs are fixed.
>
> In theory we could fdatasync/fsync every input file on every platform.
> It'd be a shame to do that, though; that would slow down everybody
> merely to work around this ZFS bug.
>
> > Also it seems like fsync() does avoid the ZFS issue as mentioned in:
> > https://github.com/openzfs/zfs/issues/11900
>
> Yes. I'm hoping that fdatasync suffices as it's lighter-weight. But if
> fsync is needed we can use fsync.
>
> > BTW I'm slightly worried about retrying SEEK_DATA as
> > FreeBSD 9.1 has a bug with large sparse files at least
> > where it takes ages for SEEK_DATA to return:
> >     36.13290615 lseek(3,0x0,SEEK_DATA)         = -32768 (0xffff8000)
> > If ENXIO is not set in that case, then there is no issue.
>
> Wait - lseek returns a number less than -1?! We could easily check for
> that FreeBSD bug, perhaps as an independent patch; this shouldn't
> require any extra syscalls.
>
> Also please see
> <https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=256205>. It appears
> that ZFS has significant bugs in this area on FreeBSD, bugs that haven't
> been fixed yet. That bug report does suggest that an fsync (and I hope
> fdatasync) works around the bugs.
>
> > Also I'm not sure restricting sync to ENXIO is general enough,
> > as an strace from a problematic cp, from the github issue above is:
> >     lseek(3, 0, SEEK_DATA)            = 0
> >     lseek(3, 0, SEEK_HOLE)            = 131072
> >     lseek(3, 0, SEEK_SET)             = 0
> >     read(3, "\177ELF\2\1"..., 131072) = 131072
> >     write(4, "\177ELF\2\"..., 131072) = 131072
> >     lseek(3, 131072, SEEK_DATA)       = -1 ENXIO
> >     ftruncate(4, 3318813)             = 0
>
> How about if we also do an fdatasync+retry after that 2nd lseek yields
> ENXIO? Would that suffice to work around the ZFS bug? Would it be too
> much of a performance penalty for non-ZFS users?

I don't think there is anything special about first or second lseek().
ZFS just seems to be returning ENXIO depending on the write pattern of the file.
Since we currently always have an expected SEEK_DATA ENXIO at end of file,
doing a sync on any ENXIO would be best avoided.

I wonder after getting a SEEK_DATA ENXIO, might we correlate
we really are at end of file by checking SEEK_HOLE also returns ENXIO?
If not we could do the datasync and try SEEK_DATA again.
I've not seen enough syscall traces to be confident of that though.

BTW we only attempt the SEEK_HOLE scanning when our heuristic
of st_blocks being less than st_size indicates there may be holes.
It's a bit surprising that that is the case for these elf binaries.
Perhaps zfs is updating st_blocks async, or perhaps there are
runs of zeros in these files that a linker or whatever is sparsifying?

cheers,
Pádraig