From 9d87b08c5279b2f0769dc7a788a9d7296926b2a2 Mon Sep 17 00:00:00 2001 From: Luflosi Date: Wed, 26 Jan 2022 23:35:54 +0100 Subject: [PATCH 1/2] Fix logic error in file copy helper functions If `errno` is `EAGAIN`, the second if statement is never true and if `errno` is not `EAGAIN`, the function returns immediately, not checking the second if statement. This commit fixes this. A logically equivalent change would have been to simply swap the two if statements but I find this version to be more readable. --- kitty/fast-file-copy.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/kitty/fast-file-copy.c b/kitty/fast-file-copy.c index dec635286..15f0257fe 100644 --- a/kitty/fast-file-copy.c +++ b/kitty/fast-file-copy.c @@ -60,9 +60,9 @@ copy_with_sendfile(int infd, int outfd, off_t in_pos, size_t len, FastFileCopyBu off_t r = in_pos; ssize_t n = sendfile(outfd, infd, &r, len); if (n < 0) { - if (errno != EAGAIN) return false; + if (errno == EAGAIN) continue; if (errno == ENOSYS || errno == EPERM) return copy_with_buffer(infd, outfd, in_pos, len, fcb); - continue; + return false; } if (n == 0) { // happens if input file is truncated @@ -83,9 +83,9 @@ copy_with_file_range(int infd, int outfd, off_t in_pos, size_t len, FastFileCopy off64_t r = in_pos; ssize_t n = copy_file_range(infd, &r, outfd, NULL, len, 0); if (n < 0) { - if (errno != EAGAIN) return false; + if (errno == EAGAIN) continue; if (errno == ENOSYS || errno == EPERM) return copy_with_sendfile(infd, outfd, in_pos, len, fcb); - continue; + return false; } if (n == 0) { // happens if input file is truncated From 4c78a50dcf98dbc66b5111a36d48a000baefd76f Mon Sep 17 00:00:00 2001 From: Luflosi Date: Wed, 26 Jan 2022 23:39:46 +0100 Subject: [PATCH 2/2] Add more copy_file_range() and sendfile() errno exceptions On ZFS `copy_file_range()` and `sendfile()` fail for some reason, so add `EINVAL` to the list of error codes for which the code will fall back to a slower implementation. While we are at it, add all the error codes from https://go.dev/src/internal/poll/copy_file_range_linux.go for different filesystems and older kernel versions to the list of error codes from `copy_file_range()`. Without this, the cache defragmentation and the test testing it (`test_disk_cache`) fail with the error message "Failed to copy data to new disk cache file during defrag: Invalid argument" when the cache directory is stored on ZFS. Perhaps this is caused by me running ZFS 2.1.2 on Linux 5.16.0, which is not a supported combination. This ZFS version is only "compatible with 3.10 - 5.15 kernels" according to the release notes. Or maybe it's just not implemented. To reproduce the problem, execute `KITTY_CACHE_DIRECTORY=/path/to/zfs python3 test.py` with `KITTY_CACHE_DIRECTORY` pointing to a directory on ZFS. --- kitty/fast-file-copy.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/kitty/fast-file-copy.c b/kitty/fast-file-copy.c index 15f0257fe..8c3e032df 100644 --- a/kitty/fast-file-copy.c +++ b/kitty/fast-file-copy.c @@ -61,7 +61,10 @@ copy_with_sendfile(int infd, int outfd, off_t in_pos, size_t len, FastFileCopyBu ssize_t n = sendfile(outfd, infd, &r, len); if (n < 0) { if (errno == EAGAIN) continue; - if (errno == ENOSYS || errno == EPERM) return copy_with_buffer(infd, outfd, in_pos, len, fcb); + if (errno == ENOSYS || // No kernel support + errno == EPERM || + errno == EINVAL) // ZFS for some reason + return copy_with_buffer(infd, outfd, in_pos, len, fcb); return false; } if (n == 0) { @@ -84,7 +87,13 @@ copy_with_file_range(int infd, int outfd, off_t in_pos, size_t len, FastFileCopy ssize_t n = copy_file_range(infd, &r, outfd, NULL, len, 0); if (n < 0) { if (errno == EAGAIN) continue; - if (errno == ENOSYS || errno == EPERM) return copy_with_sendfile(infd, outfd, in_pos, len, fcb); + if (errno == ENOSYS || // Linux < 4.5 + errno == EPERM || // Possibly Docker + errno == EINVAL || // ZFS for some reason + errno == EIO || // CIFS + errno == EOPNOTSUPP || // NFS + errno == EXDEV) // Prior to Linux 5.3, it was not possible to copy_file_range across file systems + return copy_with_sendfile(infd, outfd, in_pos, len, fcb); return false; } if (n == 0) {