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

From: Paul Eggert <[email-masked]>
Date: Thu, 28 Oct 2021 00:56:11 -0700

On 10/27/21 03:00, Janne Heß wrote:
> Building another package (peertube) on x86_64-linux on ext4 also fails with strange errors in the
> test suite, something about "Error: The service is no longer running". This does not happen when the mentioned
> coreutils commit is undone by replacing #ifdef with #if 0 [3].

So the problem is not limited to ZFS? Which means that even if we 
implemented Pádraig's suggestion and disabled SEEK_HOLE on zfs, we'd 
still run into problems? That's really puzzling. Particularly since it's 
not clear what program is generating the diagnostic "The service is no 
longer running", or how it's related to GNU cp.

Anyway, the ZFS issue sounds like a serious bug in lseek+SEEK_DATA that 
really needs to be fixed. This is not just a coreutils issue, as other 
programs use SEEK_DATA.

I assume the ZFS bug (if the bug is related to ZFS, anyway) is a race 
condition of some sort; at least, that's what the trace in 
<https://github.com/openzfs/zfs/issues/11900> suggests.

In particular, I was struck that the depthcharge.config file that 'cp' 
was reading from was created by some other process, this way:

[pid 3014182] openat(AT_FDCWD, 
"/build/guybrush/tmp/portage/sys-boot/depthcharge-0.0.1-r3237/image/firmware/guybrush/depthcharge/depthcharge.config", 
O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 0666) = 4
[pid 3014182] fstat(4, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
[pid 3014182] ioctl(4, TCGETS, 0x7ffd919d61c0) = -1 ENOTTY 
(Inappropriate ioctl for device)
[pid 3014182] lseek(3, 0, SEEK_CUR)     = 0
[pid 3014182] lseek(3, 0, SEEK_DATA)    = 0
[pid 3014182] lseek(3, 0, SEEK_HOLE)    = 9608
[pid 3014182] copy_file_range(3, [0], 4, [0], 9608, 0) = 9608
[pid 3014182] lseek(3, 0, SEEK_CUR)     = 9608
[pid 3014182] lseek(3, 9608, SEEK_DATA) = -1 ENXIO (No such device or 
address)
[pid 3014182] lseek(3, 0, SEEK_END)     = 9608
[pid 3014182] ftruncate(4, 9608)        = 0
[pid 3014182] close(4)                  = 0

So, one hypothesis is that ZFS's implementation of copy_file_range does 
not set up data structures appropriately for cp's later use of 
lseek+SEEK_DATA when reading depthcharge.config. That is, from cp's 
point of view, the ftruncate(4, 9608) has been executed but the 
copy_file_range(3, [0], 4, [0], 9608, 0) has not been executed yet (it's 
cached somewhere, no doubt).

If my guess is right, then an fdatasync or fsync on cp's input might 
work around  common instances of this ZFS bug. Could you try the 
attached coreutils patch, and see whether it works around the bug? Or 
perhaps change 'fdatasync' with 'fsync' in the attached patch? Thanks.

From 451c15fcb72e09afba499f188eb6e03ced49b1ca Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Thu, 28 Oct 2021 00:41:55 -0700
Subject: [PATCH] cp: attempt to work around ZFS bug

* src/copy.c (infer_scantype): Attempt a partial workaround for a
Linux file system bug (see Bug#51433).
---
 src/copy.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/copy.c b/src/copy.c
index cb9018f93..6f4566d9f 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1097,6 +1097,16 @@ infer_scantype (int fd, struct stat const *sb,
 
 #ifdef SEEK_HOLE
   scan_inference->ext_start = lseek (fd, 0, SEEK_DATA);
+
+  /* If lseek fails with ENXIO, it may be a Linux file system bug
+     <https://bugs.gnu.org/51433>.  Try to work around it by
+     fdatasyncing the input and then retrying the lseek.  */
+  if (scan_inference->ext_start < 0 && errno == ENXIO)
+    {
+      fdatasync (fd);
+      scan_inference->ext_start = lseek (fd, 0, SEEK_DATA);
+    }
+
   if (0 <= scan_inference->ext_start)
     return LSEEK_SCANTYPE;
   else if (errno != EINVAL && !is_ENOTSUP (errno))
-- 
2.31.1