{"thread":{"id":"66012","subject":"[PATCH 5/5] fast-import: use writev(3p) to send cat-blob responses","startedAt":"2026-07-16T07:52:53Z","lastAt":"2026-08-07T06:30:07Z","messageCount":27,"participants":["Patrick Steinhardt","Simon Richter","Johannes Sixt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"548374","messageId":"20260716-pks-reintroduce-writev-v1-1-ea9038c884bc@pks.im","threadId":"66012","inReplyTo":"20260716-pks-reintroduce-writev-v1-0-ea9038c884bc@pks.im","subject":"[PATCH 1/5] compat/posix: introduce writev(3p) wrapper","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-07-16T07:52:19Z","receivedAt":"2026-07-16T07:52:44Z","isPatch":true,"body":"In a subsequent commit we're going to add the first caller to\nwritev(3p). Introduce a compatibility wrapper for this syscall that we\ncan use on systems that don't have this syscall.\n\nThe syscall exists on modern Unixes like Linux and macOS, and seemingly\neven for NonStop according to [1]. It doesn't seem to exist on Windows\nthough.\n\n[1]: http://nonstoptools.com/manuals/OSS-SystemCalls.pdf\n[2]: https://www.gnu.org/software/gnulib/manual/html_node/writev.html\n\nHelped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Makefile                            |  4 ++++\n compat/posix.h                      | 14 ++++++++++++\n compat/writev.c                     | 44 +++++++++++++++++++++++++++++++++++++\n config.mak.uname                    |  2 ++\n contrib/buildsystems/CMakeLists.txt |  6 ++++-\n meson.build                         |  1 +\n 6 files changed, 70 insertions(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex 1f3f099f5c..eda5ecc5b4 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2033,6 +2033,10 @@ ifdef NO_PREAD\n \tCOMPAT_CFLAGS += -DNO_PREAD\n \tCOMPAT_OBJS += compat/pread.o\n endif\n+ifdef NO_WRITEV\n+\tCOMPAT_CFLAGS += -DNO_WRITEV\n+\tCOMPAT_OBJS += compat/writev.o\n+endif\n ifdef NO_FAST_WORKING_DIRECTORY\n \tBASIC_CFLAGS += -DNO_FAST_WORKING_DIRECTORY\n endif\ndiff --git a/compat/posix.h b/compat/posix.h\nindex e2e794cad7..71cc731620 100644\n--- a/compat/posix.h\n+++ b/compat/posix.h\n@@ -148,6 +148,9 @@\n #include <sys/socket.h>\n #include <sys/ioctl.h>\n #include <sys/statvfs.h>\n+#ifndef NO_WRITEV\n+#include <sys/uio.h>\n+#endif\n #include <termios.h>\n #ifndef NO_SYS_SELECT_H\n #include <sys/select.h>\n@@ -334,6 +337,17 @@ int git_lstat(const char *, struct stat *);\n ssize_t git_pread(int fd, void *buf, size_t count, off_t offset);\n #endif\n \n+#ifdef NO_WRITEV\n+#define writev git_writev\n+#define iovec git_iovec\n+struct git_iovec {\n+\tvoid *iov_base;\n+\tsize_t iov_len;\n+};\n+\n+ssize_t git_writev(int fd, const struct iovec *iov, int iovcnt);\n+#endif\n+\n #ifdef NO_SETENV\n #define setenv gitsetenv\n int gitsetenv(const char *, const char *, int);\ndiff --git a/compat/writev.c b/compat/writev.c\nnew file mode 100644\nindex 0000000000..ab2e223634\n--- /dev/null\n+++ b/compat/writev.c\n@@ -0,0 +1,44 @@\n+#include \"../git-compat-util.h\"\n+#include \"../wrapper.h\"\n+\n+ssize_t git_writev(int fd, const struct iovec *iov, int iovcnt)\n+{\n+\tsize_t total_written = 0;\n+\tsize_t sum = 0;\n+\n+\t/*\n+\t * According to writev(3p), the syscall shall error with EINVAL in case\n+\t * the sum of `iov_len` overflows `ssize_t`.\n+\t */\n+\tfor (int i = 0; i < iovcnt; i++) {\n+\t\tif (iov[i].iov_len > maximum_signed_value_of_type(ssize_t) ||\n+\t\t    iov[i].iov_len + sum > maximum_signed_value_of_type(ssize_t)) {\n+\t\t\terrno = EINVAL;\n+\t\t\treturn -1;\n+\t\t}\n+\n+\t\tsum += iov[i].iov_len;\n+\t}\n+\n+\tfor (int i = 0; i < iovcnt; i++) {\n+\t\tconst char *bytes = iov[i].iov_base;\n+\t\tsize_t iovec_written = 0;\n+\n+\t\twhile (iovec_written < iov[i].iov_len) {\n+\t\t\tssize_t bytes_written = xwrite(fd, bytes + iovec_written,\n+\t\t\t\t\t\t       iov[i].iov_len - iovec_written);\n+\t\t\tif (bytes_written < 0) {\n+\t\t\t\tif (total_written)\n+\t\t\t\t\tgoto out;\n+\t\t\t\treturn bytes_written;\n+\t\t\t}\n+\t\t\tif (!bytes_written)\n+\t\t\t\tgoto out;\n+\t\t\tiovec_written += bytes_written;\n+\t\t\ttotal_written += bytes_written;\n+\t\t}\n+\t}\n+\n+out:\n+\treturn (ssize_t) total_written;\n+}\ndiff --git a/config.mak.uname b/config.mak.uname\nindex 9ebd240378..95ef6e64dc 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -483,6 +483,7 @@ ifeq ($(uname_S),Windows)\n \tSANE_TOOL_PATH ?= $(msvc_bin_dir_msys)\n \tHAVE_ALLOCA_H = YesPlease\n \tNO_PREAD = YesPlease\n+\tNO_WRITEV = YesPlease\n \tNEEDS_CRYPTO_WITH_SSL = YesPlease\n \tNO_LIBGEN_H = YesPlease\n \tNO_POLL = YesPlease\n@@ -697,6 +698,7 @@ ifeq ($(uname_S),MINGW)\n \tpathsep = ;\n \tHAVE_ALLOCA_H = YesPlease\n \tNO_PREAD = YesPlease\n+\tNO_WRITEV = YesPlease\n \tNEEDS_CRYPTO_WITH_SSL = YesPlease\n \tNO_LIBGEN_H = YesPlease\n \tNO_POLL = YesPlease\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex a57c4b464f..8f56203f34 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -378,7 +378,7 @@ endif()\n #function checks\n set(function_checks\n \tstrcasestr memmem strlcpy strtoimax strtoumax strtoull\n-\tsetenv mkdtemp poll pread memmem)\n+\tsetenv mkdtemp poll pread memmem writev)\n \n #unsetenv,hstrerror are incompatible with windows build\n if(NOT WIN32)\n@@ -423,6 +423,10 @@ if(NOT HAVE_MEMMEM)\n \tlist(APPEND compat_SOURCES compat/memmem.c)\n endif()\n \n+if(NOT HAVE_WRITEV)\n+\tlist(APPEND compat_SOURCES compat/writev.c)\n+endif()\n+\n if(NOT WIN32)\n \tif(NOT HAVE_UNSETENV)\n \t\tlist(APPEND compat_SOURCES compat/unsetenv.c)\ndiff --git a/meson.build b/meson.build\nindex ca235801cf..613828ff25 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -1446,6 +1446,7 @@ checkfuncs = {\n   'initgroups' : [],\n   'strtoumax' : ['strtoumax.c', 'strtoimax.c'],\n   'pread' : ['pread.c'],\n+  'writev' : ['writev.c'],\n }\n \n if host_machine.system() == 'windows'\n\n-- \n2.55.0.313.g8d093f411d.dirty\n\n"},{"id":"548377","messageId":"20260716-pks-reintroduce-writev-v1-0-ea9038c884bc@pks.im","threadId":"66012","inReplyTo":null,"subject":"[PATCH 0/5] Reintroduce writev(3p)","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-07-16T07:52:18Z","receivedAt":"2026-07-16T07:52:44Z","isPatch":true,"body":"Hi,\n\nthis patch series reintroduces the writev(3p) wrapper. This wrapper was\noriginally introduced as part of Git 2.54 [1], but was ejected due to\nissues on NonStop [2].\n\nThis patch series here revives the effort with a couple of fixes on top:\n\n  - It picks Dscho's fix for CMake [3].\n\n  - It picks a fix for NonStop [4] and polishes it a bit.\n\n  - It adapts one more site to demonstrate that its usefulness is not\n    limited to a single callsite, only.\n\nFurthermore, I have included benchmarks now that demonstrate the\nbenefits to make this series a bit more appealing. Ultimately, I'd be\nfine if we say we rather don't want to go this way though. I merely\nwanted to tie some loose ends that I left dangling.\n\nThat, and it's nice to not work on pluggable object databases once in a\nwhile.\n\nThanks!\n\nPatrick\n\n[1]: <20260227-pks-upload-pack-write-contention-v1-0-7166fe255704@pks.im>\n[2]: <028901dcc859$d2419470$76c4bd50$@nexbridge.com>\n[3]: <pull.2078.git.1775206502134.gitgitgadget@gmail.com>\n[4]: <20260409-b4-pks-writev-max-io-size-v1-1-81730e8f35df@pks.im>\n\n---\nPatrick Steinhardt (5):\n      compat/posix: introduce writev(3p) wrapper\n      wrapper: introduce writev(3p) wrappers\n      wrapper: properly handle MAX_IO_SIZE in writev(3p)\n      sideband: use writev(3p) to send pktlines\n      fast-import: use writev(3p) to send cat-blob responses\n\n Makefile                            |  4 ++\n builtin/fast-import.c               | 18 +++++++--\n compat/posix.h                      | 14 +++++++\n compat/writev.c                     | 44 +++++++++++++++++++++\n config.mak.uname                    |  2 +\n contrib/buildsystems/CMakeLists.txt |  6 ++-\n meson.build                         |  1 +\n sideband.c                          | 14 +++++--\n wrapper.c                           | 78 +++++++++++++++++++++++++++++++++++++\n wrapper.h                           | 10 +++++\n write-or-die.c                      |  8 ++++\n write-or-die.h                      |  1 +\n 12 files changed, 193 insertions(+), 7 deletions(-)\n\n\n---\nbase-commit: 55526a18268bbc1ddaf8a6b7850c33d984eac9e9\nchange-id: 20260714-pks-reintroduce-writev-2d8f7e52eee9\n\n"},{"id":"548376","messageId":"20260716-pks-reintroduce-writev-v1-2-ea9038c884bc@pks.im","threadId":"66012","inReplyTo":"20260716-pks-reintroduce-writev-v1-0-ea9038c884bc@pks.im","subject":"[PATCH 2/5] wrapper: introduce writev(3p) wrappers","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-07-16T07:52:20Z","receivedAt":"2026-07-16T07:52:46Z","isPatch":true,"body":"In the preceding commit we have added a compatibility wrapper for the\nwritev(3p) syscall. Introduce some generic wrappers for this function\nthat we nowadays take for granted in the Git codebase.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n wrapper.c      | 41 +++++++++++++++++++++++++++++++++++++++++\n wrapper.h      |  9 +++++++++\n write-or-die.c |  8 ++++++++\n write-or-die.h |  1 +\n 4 files changed, 59 insertions(+)\n\ndiff --git a/wrapper.c b/wrapper.c\nindex 16f5a63fbb..be8fa575e6 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -323,6 +323,47 @@ ssize_t write_in_full(int fd, const void *buf, size_t count)\n \treturn total;\n }\n \n+ssize_t writev_in_full(int fd, struct iovec *iov, int iovcnt)\n+{\n+\tssize_t total_written = 0;\n+\n+\twhile (iovcnt) {\n+\t\tssize_t bytes_written = writev(fd, iov, iovcnt);\n+\t\tif (bytes_written < 0) {\n+\t\t\tif (errno == EINTR || errno == EAGAIN)\n+\t\t\t\tcontinue;\n+\t\t\treturn -1;\n+\t\t}\n+\t\tif (!bytes_written) {\n+\t\t\terrno = ENOSPC;\n+\t\t\treturn -1;\n+\t\t}\n+\n+\t\ttotal_written += bytes_written;\n+\n+\t\t/*\n+\t\t * We first need to discard any iovec entities that have been\n+\t\t * fully written.\n+\t\t */\n+\t\twhile (iovcnt && (size_t)bytes_written >= iov->iov_len) {\n+\t\t\tbytes_written -= iov->iov_len;\n+\t\t\tiov++;\n+\t\t\tiovcnt--;\n+\t\t}\n+\n+\t\t/*\n+\t\t * Finally, we need to adjust the last iovec in case we have\n+\t\t * performed a partial write.\n+\t\t */\n+\t\tif (iovcnt && bytes_written) {\n+\t\t\tiov->iov_base = (char *) iov->iov_base + bytes_written;\n+\t\t\tiov->iov_len -= bytes_written;\n+\t\t}\n+\t}\n+\n+\treturn total_written;\n+}\n+\n ssize_t pread_in_full(int fd, void *buf, size_t count, off_t offset)\n {\n \tchar *p = buf;\ndiff --git a/wrapper.h b/wrapper.h\nindex 15ac3bab6e..27519b32d1 100644\n--- a/wrapper.h\n+++ b/wrapper.h\n@@ -47,6 +47,15 @@ ssize_t read_in_full(int fd, void *buf, size_t count);\n ssize_t write_in_full(int fd, const void *buf, size_t count);\n ssize_t pread_in_full(int fd, void *buf, size_t count, off_t offset);\n \n+/*\n+ * Try to write all iovecs. Returns -1 in case an error occurred with a proper\n+ * errno set, the number of bytes written otherwise.\n+ *\n+ * Note that the iovec will be modified as a result of this call to adjust for\n+ * partial writes!\n+ */\n+ssize_t writev_in_full(int fd, struct iovec *iov, int iovcnt);\n+\n static inline ssize_t write_str_in_full(int fd, const char *str)\n {\n \treturn write_in_full(fd, str, strlen(str));\ndiff --git a/write-or-die.c b/write-or-die.c\nindex 01a9a51fa2..5f522fb728 100644\n--- a/write-or-die.c\n+++ b/write-or-die.c\n@@ -96,6 +96,14 @@ void write_or_die(int fd, const void *buf, size_t count)\n \t}\n }\n \n+void writev_or_die(int fd, struct iovec *iov, int iovlen)\n+{\n+\tif (writev_in_full(fd, iov, iovlen) < 0) {\n+\t\tcheck_pipe(errno);\n+\t\tdie_errno(\"writev error\");\n+\t}\n+}\n+\n void fwrite_or_die(FILE *f, const void *buf, size_t count)\n {\n \tif (fwrite(buf, 1, count, f) != count)\ndiff --git a/write-or-die.h b/write-or-die.h\nindex ff0408bd84..a045bdfaef 100644\n--- a/write-or-die.h\n+++ b/write-or-die.h\n@@ -7,6 +7,7 @@ void fprintf_or_die(FILE *, const char *fmt, ...);\n void fwrite_or_die(FILE *f, const void *buf, size_t count);\n void fflush_or_die(FILE *f);\n void write_or_die(int fd, const void *buf, size_t count);\n+void writev_or_die(int fd, struct iovec *iov, int iovlen);\n \n /*\n  * These values are used to help identify parts of a repository to fsync.\n\n-- \n2.55.0.313.g8d093f411d.dirty\n\n"},{"id":"548375","messageId":"20260716-pks-reintroduce-writev-v1-3-ea9038c884bc@pks.im","threadId":"66012","inReplyTo":"20260716-pks-reintroduce-writev-v1-0-ea9038c884bc@pks.im","subject":"[PATCH 3/5] wrapper: properly handle MAX_IO_SIZE in writev(3p)","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-07-16T07:52:21Z","receivedAt":"2026-07-16T07:52:47Z","isPatch":true,"body":"Some systems like NonStop set a comparatively small `MAX_IO_SIZE`, which\nlimits the maximum number of bytes we're allowed to write in a single\ncall. We already handle this limit properly in `xwrite()`, but we have\nrecently introduced wrappers for writev(3p) where we don't. This will\ncause the syscall to return EINVAL in case somebody passes an iovec\nentry to writev(3p) that is larger than `MAX_IO_SIZE`.\n\nIntroduce a new function `xwritev()` that is similar to `xwrite()` in\nthat it handles such platform-specific nuances:\n\n  - We only pass the leading iovec entries to writev(3p) that fit into\n    `MAX_IO_SIZE`, pretending that the underlying syscall performed a\n    short write. This mirrors how `xwrite()` chomps overly large\n    requests before handing them to write(3p). As a consequence, callers\n    will never see writev(3p)'s EINVAL error for requests whose summed\n    length would overflow an ssize_t, but observe a short write instead.\n\n  - If already the first iovec entry exceeds the limit we instead punt\n    to `xwrite()`, which knows to handle this case for us.\n\n  - We restart the underlying syscall on EINTR and EAGAIN, just like\n    `xwrite()` does for write(3p).\n\nAdapt `writev_in_full()` to use this new wrapper. With the retry logic\nnow living in `xwritev()`, the calling loop becomes the exact mirror\nimage of `write_in_full()`, which also retains the responsibility of\ntranslating a zero-length write into ENOSPC.\n\nReported-by: Randall Becker <randall.becker@nexbridge.ca>\nHelped-by: Jeff King <peff@peff.net>\nHelped-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n wrapper.c | 47 ++++++++++++++++++++++++++++++++++++++++++-----\n wrapper.h |  1 +\n 2 files changed, 43 insertions(+), 5 deletions(-)\n\ndiff --git a/wrapper.c b/wrapper.c\nindex be8fa575e6..561f9ee9c9 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -323,17 +323,54 @@ ssize_t write_in_full(int fd, const void *buf, size_t count)\n \treturn total;\n }\n \n+ssize_t xwritev(int fd, struct iovec *iov, int iovcnt)\n+{\n+\tsize_t allowed = MAX_IO_SIZE;\n+\tint i;\n+\n+\t/*\n+\t * Some platforms define a comparatively small `MAX_IO_SIZE` that\n+\t * limits how many bytes can be written with a single call to\n+\t * write(3p) or writev(3p); exceeding that limit causes the syscall to\n+\t * fail with EINVAL. Just like xwrite() chomps overly large requests\n+\t * for write(3p), pretend that the underlying writev(3p) performed a\n+\t * short write by only passing along the leading iovec entries that\n+\t * fit into that limit.\n+\t */\n+\tfor (i = 0; i < iovcnt; i++) {\n+\t\tif (iov[i].iov_len > allowed) {\n+\t\t\t/*\n+\t\t\t * If the first buffer is larger than MAX_IO_SIZE,\n+\t\t\t * let xwrite() deal with it.\n+\t\t\t */\n+\t\t\tif (!i)\n+\t\t\t\treturn xwrite(fd, iov->iov_base, iov->iov_len);\n+\t\t\tbreak;\n+\t\t}\n+\t\tallowed -= iov[i].iov_len;\n+\t}\n+\n+\twhile (1) {\n+\t\tssize_t bytes_written = writev(fd, iov, i);\n+\t\tif (bytes_written < 0) {\n+\t\t\tif (errno == EINTR)\n+\t\t\t\tcontinue;\n+\t\t\tif (handle_nonblock(fd, POLLOUT, errno))\n+\t\t\t\tcontinue;\n+\t\t}\n+\n+\t\treturn bytes_written;\n+\t}\n+}\n+\n ssize_t writev_in_full(int fd, struct iovec *iov, int iovcnt)\n {\n \tssize_t total_written = 0;\n \n \twhile (iovcnt) {\n-\t\tssize_t bytes_written = writev(fd, iov, iovcnt);\n-\t\tif (bytes_written < 0) {\n-\t\t\tif (errno == EINTR || errno == EAGAIN)\n-\t\t\t\tcontinue;\n+\t\tssize_t bytes_written = xwritev(fd, iov, iovcnt);\n+\t\tif (bytes_written < 0)\n \t\t\treturn -1;\n-\t\t}\n \t\tif (!bytes_written) {\n \t\t\terrno = ENOSPC;\n \t\t\treturn -1;\ndiff --git a/wrapper.h b/wrapper.h\nindex 27519b32d1..a6287d7f4d 100644\n--- a/wrapper.h\n+++ b/wrapper.h\n@@ -16,6 +16,7 @@ void *xmmap_gently(void *start, size_t length, int prot, int flags, int fd, off_\n int xopen(const char *path, int flags, ...);\n ssize_t xread(int fd, void *buf, size_t len);\n ssize_t xwrite(int fd, const void *buf, size_t len);\n+ssize_t xwritev(int fd, struct iovec *iov, int iovcnt);\n ssize_t xpread(int fd, void *buf, size_t len, off_t offset);\n int xdup(int fd);\n FILE *xfopen(const char *path, const char *mode);\n\n-- \n2.55.0.313.g8d093f411d.dirty\n\n"},{"id":"548373","messageId":"20260716-pks-reintroduce-writev-v1-4-ea9038c884bc@pks.im","threadId":"66012","inReplyTo":"20260716-pks-reintroduce-writev-v1-0-ea9038c884bc@pks.im","subject":"[PATCH 4/5] sideband: use writev(3p) to send pktlines","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-07-16T07:52:22Z","receivedAt":"2026-07-16T07:52:50Z","isPatch":true,"body":"Every pktline that we send out via `send_sideband()` currently requires\ntwo syscalls: one to write the pktline's length, and one to send its\ndata. This typically isn't all that much of a problem, but under extreme\nload the syscalls may cause contention in the kernel.\n\nRefactor the code to instead use the newly introduced writev(3p) infra\nso that we can send out the data with a single syscall. This reduces the\nnumber of syscalls from around 133,000 calls to write(3p) to around\n67,000 calls to writev(3p).\n\nThis change leads to a performance improvement for git-upload-pack(1),\nbut we have to cheat a bit to really make it measurable. Usually, the\ntime is strongly dominated by generating the packfile itself. But if we\nprecompute the pack and serve it via the pack-objects hook then we can\nessentially eliminate that overhead. The following setup is executed in\nthe Git repository:\n\n  $ cat >request <<-EOF\n  0048want 5ce91c059e41090e7d2cffad39c04af8acf98dc1 side-band no-progress\n  00000009done\n  EOF\n  $ echo 5ce91c059e41090e7d2cffad39c04af8acf98dc1 | git pack-objects --revs --stdout >pack\n  $ cat >hook <<-EOF\n  #!/bin/sh\n  cat >/dev/null\n  cat \"$(pwd)\"/pack\n  EOF\n  $ chmod u+x hook\n  $ git -c uploadpack.packObjectsHook=\"$(pwd)\"/hook upload-pack . <request\n\nBenchmarking the last command leads to the following results:\n\n  Benchmark 1: HEAD~\n    Time (mean ± σ):     192.9 ms ±   0.6 ms    [User: 106.5 ms, System: 95.3 ms]\n    Range (min … max):   191.7 ms … 194.1 ms    50 runs\n\n  Benchmark 2: HEAD\n    Time (mean ± σ):     141.1 ms ±   0.7 ms    [User: 63.2 ms, System: 86.6 ms]\n    Range (min … max):   139.8 ms … 142.7 ms    50 runs\n\n  Summary\n    HEAD ran\n      1.37 ± 0.01 times faster than HEAD~\n\nThis might not be impressive in absolute numbers when you also take into\naccount the time it takes to generate the packfile itself. But GitLab\n(and supposedly other forges) have caching mechanisms in place that work\nexactly like the above setup, where repeated incoming requests can be\nserved from the same cached packfile. And in those cases, the impact is\nsizeable.\n\nMore importantly though, as hinted at above, GitLab has observed in the\npast that with enough cache hits we eventually start to saturate a\nsemaphore in the Linux kernel itself in the pipe write path. This\nbottleneck is being moved a bit by having to do less syscalls.\n\nSuggested-by: Jeff King <peff@peff.net>\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n sideband.c | 14 +++++++++++---\n 1 file changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git a/sideband.c b/sideband.c\nindex 1523a53e1d..94e5b56172 100644\n--- a/sideband.c\n+++ b/sideband.c\n@@ -441,6 +441,7 @@ void send_sideband(int fd, int band, const char *data, ssize_t sz, int packet_ma\n \tconst char *p = data;\n \n \twhile (sz) {\n+\t\tstruct iovec iov[2];\n \t\tunsigned n;\n \t\tchar hdr[5];\n \n@@ -450,12 +451,19 @@ void send_sideband(int fd, int band, const char *data, ssize_t sz, int packet_ma\n \t\tif (0 <= band) {\n \t\t\txsnprintf(hdr, sizeof(hdr), \"%04x\", n + 5);\n \t\t\thdr[4] = band;\n-\t\t\twrite_or_die(fd, hdr, 5);\n+\t\t\tiov[0].iov_base = hdr;\n+\t\t\tiov[0].iov_len = 5;\n \t\t} else {\n \t\t\txsnprintf(hdr, sizeof(hdr), \"%04x\", n + 4);\n-\t\t\twrite_or_die(fd, hdr, 4);\n+\t\t\tiov[0].iov_base = hdr;\n+\t\t\tiov[0].iov_len = 4;\n \t\t}\n-\t\twrite_or_die(fd, p, n);\n+\n+\t\tiov[1].iov_base = (void *) p;\n+\t\tiov[1].iov_len = n;\n+\n+\t\twritev_or_die(fd, iov, ARRAY_SIZE(iov));\n+\n \t\tp += n;\n \t\tsz -= n;\n \t}\n\n-- \n2.55.0.313.g8d093f411d.dirty\n\n"},{"id":"548372","messageId":"20260716-pks-reintroduce-writev-v1-5-ea9038c884bc@pks.im","threadId":"66012","inReplyTo":"20260716-pks-reintroduce-writev-v1-0-ea9038c884bc@pks.im","subject":"[PATCH 5/5] fast-import: use writev(3p) to send cat-blob responses","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-07-16T07:52:23Z","receivedAt":"2026-07-16T07:52:53Z","isPatch":true,"body":"When answering a `cat-blob` command, `cat_blob()` issues three separate\ncalls to write(3p) on the cat-blob fd: one for the header line, one for\nthe full blob payload, and one for the trailing newline. Frontends like\ngit-filter-repo issue these commands in bulk, once per rewritten blob,\nso the syscall overhead adds up.\n\nUse `writev_in_full()` to send all three parts with a single syscall.\n\nThis can be benchmarked with the following setup:\n\n    $ git cat-file --unordered --filter=object:type=blob\n        --batch-check='cat-blob %(objectname)' --batch-all-objects >request\n    $ git fast-import --cat-blob-fd=3 <request\n\nExecuting this with 100,000 objects in linux.git:\n\n  Benchmark 1: HEAD~\n    Time (mean ± σ):      1.320 s ±  0.003 s    [User: 1.154 s, System: 0.161 s]\n    Range (min … max):    1.314 s …  1.324 s    10 runs\n\n  Benchmark 2: HEAD\n    Time (mean ± σ):      1.270 s ±  0.022 s    [User: 1.133 s, System: 0.132 s]\n    Range (min … max):    1.209 s …  1.282 s    10 runs\n\n  Summary\n    HEAD ran\n      1.04 ± 0.02 times faster than HEAD~\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n builtin/fast-import.c | 18 +++++++++++++++---\n 1 file changed, 15 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/fast-import.c b/builtin/fast-import.c\nindex aa656c5195..48fda01c94 100644\n--- a/builtin/fast-import.c\n+++ b/builtin/fast-import.c\n@@ -3332,6 +3332,7 @@ static void cat_blob_write(const char *buf, unsigned long size)\n static void cat_blob(struct object_entry *oe, struct object_id *oid)\n {\n \tstruct strbuf line = STRBUF_INIT;\n+\tstruct iovec iov[3];\n \tunsigned long size;\n \tenum object_type type = 0;\n \tchar *buf;\n@@ -3365,10 +3366,21 @@ static void cat_blob(struct object_entry *oe, struct object_id *oid)\n \tstrbuf_reset(&line);\n \tstrbuf_addf(&line, \"%s %s %\"PRIuMAX\"\\n\", oid_to_hex(oid),\n \t\t    type_name(type), (uintmax_t)size);\n-\tcat_blob_write(line.buf, line.len);\n+\n+\t/*\n+\t * Write the header, the payload and the trailing newline with a\n+\t * single writev(3p) call instead of three separate write(3p) calls.\n+\t */\n+\tiov[0].iov_base = line.buf;\n+\tiov[0].iov_len = line.len;\n+\tiov[1].iov_base = buf;\n+\tiov[1].iov_len = size;\n+\tiov[2].iov_base = (void *) \"\\n\";\n+\tiov[2].iov_len = 1;\n+\n+\tif (writev_in_full(cat_blob_fd, iov, ARRAY_SIZE(iov)) < 0)\n+\t\tdie_errno(_(\"write to frontend failed\"));\n \tstrbuf_release(&line);\n-\tcat_blob_write(buf, size);\n-\tcat_blob_write(\"\\n\", 1);\n \tif (oe && oe->pack_id == pack_id) {\n \t\tlast_blob.offset = oe->idx.offset;\n \t\tstrbuf_attach(&last_blob.data, buf, size, size + 1);\n\n-- \n2.55.0.313.g8d093f411d.dirty\n\n"},{"id":"548379","messageId":"a2676ec6-39d5-4220-8549-10a17daec668@hogyros.de","threadId":"66012","inReplyTo":"20260716-pks-reintroduce-writev-v1-1-ea9038c884bc@pks.im","subject":"Re: [PATCH 1/5] compat/posix: introduce writev(3p) wrapper","fromName":"Simon Richter","fromEmail":"simon.richter@hogyros.de","sentAt":"2026-07-16T08:47:25Z","receivedAt":"2026-07-16T08:47:40Z","isPatch":true,"body":"Hi,\n\n> +\t\tif (iov[i].iov_len > maximum_signed_value_of_type(ssize_t) ||\n> +\t\t    iov[i].iov_len + sum > maximum_signed_value_of_type(ssize_t)) {\n\nThat feels like it could overflow.\n\n    Simon\n"},{"id":"548447","messageId":"f8050598-392f-44c9-8d66-0454740a7a12@kdbg.org","threadId":"66012","inReplyTo":"20260716-pks-reintroduce-writev-v1-0-ea9038c884bc@pks.im","subject":"Re: [PATCH 0/5] Reintroduce writev(3p)","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2026-07-16T18:56:06Z","receivedAt":"2026-07-16T18:56:12Z","isPatch":true,"body":"Am 16.07.26 um 09:52 schrieb Patrick Steinhardt:\n> this patch series reintroduces the writev(3p) wrapper. This wrapper was\n> originally introduced as part of Git 2.54 [1], but was ejected due to\n> issues on NonStop [2].\n\nPlease don't call the function \"writev\" so that nobody associates it\nwith the guarantees that only POSIX provides, but none of the\nemulations. Call it \"write_gather\", for example.\n\nAlso, clearly document that its only purpose is to reduce sequences of\nwrite() calls to a single function call, but that the additional writev\nguarantees are not needed.\n\nA range-diff to the earlier round would have been very helpful.\n\n-- Hannes\n\n"},{"id":"548451","messageId":"xmqqfr1ig0hv.fsf@gitster.g","threadId":"66012","inReplyTo":"a2676ec6-39d5-4220-8549-10a17daec668@hogyros.de","subject":"Re: [PATCH 1/5] compat/posix: introduce writev(3p) wrapper","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-16T20:09:32Z","receivedAt":"2026-07-16T20:09:36Z","isPatch":true,"body":"Simon Richter <Simon.Richter@hogyros.de> writes:\n\n> Hi,\n>\n>> +\t\tif (iov[i].iov_len > maximum_signed_value_of_type(ssize_t) ||\n>> +\t\t    iov[i].iov_len + sum > maximum_signed_value_of_type(ssize_t)) {\n>\n> That feels like it could overflow.\n\nIsn't it checking if it would overflow (and dying if so)?\n\nAh, wait.  The addition \"(iov[i].iov_len + sum)\" can indeed wrap\naround, and comparing it with the maximum value of ssize_t wouldn't\ncatch that.  Is that what you mean?\n\nWould something like this:\n\n    if (maximum_signed_value_of_type(ssize_t) < iov[i].iov_len ||\n\tiov[i].iov_len + sum < iov[i].iov_len ||\n\tmaximum_signed_value_of_type(ssize_t) < iov[i].iov_len + sum)\n\nwork better to catch the three cases independently?\n\n (1) The value is already too large on its own.\n (2) Adding them together would cause an unsigned wrap-around.\n (3) The sum does not wrap around, but it exceeds the maximum\n     representable value of ssize_t anyway.\n\n"},{"id":"548454","messageId":"xmqqwluuekbh.fsf@gitster.g","threadId":"66012","inReplyTo":"xmqqfr1ig0hv.fsf@gitster.g","subject":"Re: [PATCH 1/5] compat/posix: introduce writev(3p) wrapper","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-16T20:44:18Z","receivedAt":"2026-07-16T20:44:21Z","isPatch":true,"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Simon Richter <Simon.Richter@hogyros.de> writes:\n>\n>> Hi,\n>>\n>>> +\t\tif (iov[i].iov_len > maximum_signed_value_of_type(ssize_t) ||\n>>> +\t\t    iov[i].iov_len + sum > maximum_signed_value_of_type(ssize_t)) {\n>>\n>> That feels like it could overflow.\n>\n> Isn't it checking if it would overflow (and dying if so)?\n>\n> Ah, wait.  The addition \"(iov[i].iov_len + sum)\" can indeed wrap\n> around, and comparing it with the maximum value of ssize_t wouldn't\n> catch that.  Is that what you mean?\n>\n> Would something like this:\n>\n>     if (maximum_signed_value_of_type(ssize_t) < iov[i].iov_len ||\n> \tiov[i].iov_len + sum < iov[i].iov_len ||\n> \tmaximum_signed_value_of_type(ssize_t) < iov[i].iov_len + sum)\n>\n> work better to catch the three cases independently?\n>\n>  (1) The value is already too large on its own.\n>  (2) Adding them together would cause an unsigned wrap-around.\n>  (3) The sum does not wrap around, but it exceeds the maximum\n>      representable value of ssize_t anyway.\n\nActually, looking at it again, I think the original code is safe\nafter all, because:\n\n * \"sum\", even though it is a size_t, is checked inside the loop to\n   ensure it stays below the maximum value of ssize_t each time it\n   gets a new value.\n * iov[i].iov_len is checked to ensure it does not exceed the\n   maximum value of ssize_t by the first part of the condition.\n\nIf both values are less than or equal to the maximum value of\nssize_t, their sum is at most twice that limit.  For an N-bit\nsize_t, this sum is at most (2^N - 2), which can be computed safely\nwithout any unsigned wrap-around.\n\nSo...?\n"},{"id":"549089","messageId":"xmqqo6fso2s8.fsf@gitster.g","threadId":"66012","inReplyTo":"f8050598-392f-44c9-8d66-0454740a7a12@kdbg.org","subject":"Re: [PATCH 0/5] Reintroduce writev(3p)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-27T15:44:39Z","receivedAt":"2026-07-27T15:44:43Z","isPatch":true,"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> Am 16.07.26 um 09:52 schrieb Patrick Steinhardt:\n>> this patch series reintroduces the writev(3p) wrapper. This wrapper was\n>> originally introduced as part of Git 2.54 [1], but was ejected due to\n>> issues on NonStop [2].\n>\n> Please don't call the function \"writev\" so that nobody associates it\n> with the guarantees that only POSIX provides, but none of the\n> emulations. Call it \"write_gather\", for example.\n>\n> Also, clearly document that its only purpose is to reduce sequences of\n> write() calls to a single function call, but that the additional writev\n> guarantees are not needed.\n\nIt is philosophically more \"pure\" to have a two-level abstraction\nwhere write_gather(), which may be inspired by writev(2) but with\nspecific subset of semantics that the application needs, is used by\nthe application and have platforms with good enough writev(2) to\nimplement it in terms of it.  Other platforms may implement it\ndifferently, like a series of write(2) calls, and as long as it\nfulfills the need of write_gather(), we are OK.\n\nDoing so would also help in a minuscule way to avoid adding to the\ncomplaints we sometimes hear that our internal implementation\nassumes platform support for POSIX API and semantics way too much\neven when we do not need to.\n\nSo I do not mind going in that direction.  It feels a slightly\nroundabout approach, but in the longer run, I think it would place\nus in a much better place.\n\nI think Patrick's writev(2) follows the pattern our previous compat/\nroutines have taken.  We use real writev(2) where it is available,\nand in the fake implementations in compat/ we have comments that\nessentially say \"the real function offers X, Y, and Z, but we only\nwant X and Z and do not need Y, so this implementation does not\nsupport Y\".  It is harder to maintain because the application side\nmay be tempted over time to start depending on Y.  If some platforms\ncannot easily provide an equivalent of the real function, it is\neasier for them if the rules explicitly state from the beginning\nthat we do not require and will never require Y, needing only X and\nZ from either the fake or real implementation.\n\nAt that point, we are not describing the real function anymore, so\nyour proposal to give it a specific name is one step away from that,\nand that step is in the right direction.\n\nThanks.\n\nPS.  I was going over the list of \"waiting for response\" topics, and\nthis was one of them.  I suspect Patrick and the GitLab team are\nstill away at an offsite [*], so this is in no way poking him for an\nimmediate reroll, but rather a note sent while my attention is on\nthese stalled topics.\n\nhttps://lore.kernel.org/git/amLgMqkqxR8mKIbT@pks.im/\n"},{"id":"549659","messageId":"anL0jMyS3v2alJht@pks.im","threadId":"66012","inReplyTo":"xmqqwluuekbh.fsf@gitster.g","subject":"Re: [PATCH 1/5] compat/posix: introduce writev(3p) wrapper","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-05T08:30:04Z","receivedAt":"2026-08-05T08:30:14Z","isPatch":true,"body":"On Thu, Jul 16, 2026 at 01:44:18PM -0700, Junio C Hamano wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> > Simon Richter <Simon.Richter@hogyros.de> writes:\n> >\n> >> Hi,\n> >>\n> >>> +\t\tif (iov[i].iov_len > maximum_signed_value_of_type(ssize_t) ||\n> >>> +\t\t    iov[i].iov_len + sum > maximum_signed_value_of_type(ssize_t)) {\n> >>\n> >> That feels like it could overflow.\n> >\n> > Isn't it checking if it would overflow (and dying if so)?\n> >\n> > Ah, wait.  The addition \"(iov[i].iov_len + sum)\" can indeed wrap\n> > around, and comparing it with the maximum value of ssize_t wouldn't\n> > catch that.  Is that what you mean?\n> >\n> > Would something like this:\n> >\n> >     if (maximum_signed_value_of_type(ssize_t) < iov[i].iov_len ||\n> > \tiov[i].iov_len + sum < iov[i].iov_len ||\n> > \tmaximum_signed_value_of_type(ssize_t) < iov[i].iov_len + sum)\n> >\n> > work better to catch the three cases independently?\n> >\n> >  (1) The value is already too large on its own.\n> >  (2) Adding them together would cause an unsigned wrap-around.\n> >  (3) The sum does not wrap around, but it exceeds the maximum\n> >      representable value of ssize_t anyway.\n> \n> Actually, looking at it again, I think the original code is safe\n> after all, because:\n> \n>  * \"sum\", even though it is a size_t, is checked inside the loop to\n>    ensure it stays below the maximum value of ssize_t each time it\n>    gets a new value.\n>  * iov[i].iov_len is checked to ensure it does not exceed the\n>    maximum value of ssize_t by the first part of the condition.\n> \n> If both values are less than or equal to the maximum value of\n> ssize_t, their sum is at most twice that limit.  For an N-bit\n> size_t, this sum is at most (2^N - 2), which can be computed safely\n> without any unsigned wrap-around.\n> \n> So...?\n\nYeah, I think your analysis is correct. It's quite subtle though, so\nmaybe we should make this a bit more explicit? Something like the\nfollowing patch for example:\n\ndiff --git a/compat/writev.c b/compat/writev.c\nindex ab2e223634..960673861d 100644\n--- a/compat/writev.c\n+++ b/compat/writev.c\n@@ -12,6 +12,7 @@ ssize_t git_writev(int fd, const struct iovec *iov, int iovcnt)\n \t */\n \tfor (int i = 0; i < iovcnt; i++) {\n \t\tif (iov[i].iov_len > maximum_signed_value_of_type(ssize_t) ||\n+\t\t    unsigned_add_overflows(iov[i].iov_len, sum) ||\n \t\t    iov[i].iov_len + sum > maximum_signed_value_of_type(ssize_t)) {\n \t\t\terrno = EINVAL;\n \t\t\treturn -1;\n\nI doubt the performance overhead of this additional check is really\ngoing to matter :)\n\nPatrick\n"},{"id":"549660","messageId":"anL0nIk6uzkYR9Oc@pks.im","threadId":"66012","inReplyTo":"xmqqo6fso2s8.fsf@gitster.g","subject":"Re: [PATCH 0/5] Reintroduce writev(3p)","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-05T08:30:20Z","receivedAt":"2026-08-05T08:30:26Z","isPatch":true,"body":"On Mon, Jul 27, 2026 at 08:44:39AM -0700, Junio C Hamano wrote:\n> Johannes Sixt <j6t@kdbg.org> writes:\n> \n> > Am 16.07.26 um 09:52 schrieb Patrick Steinhardt:\n> >> this patch series reintroduces the writev(3p) wrapper. This wrapper was\n> >> originally introduced as part of Git 2.54 [1], but was ejected due to\n> >> issues on NonStop [2].\n> >\n> > Please don't call the function \"writev\" so that nobody associates it\n> > with the guarantees that only POSIX provides, but none of the\n> > emulations. Call it \"write_gather\", for example.\n> >\n> > Also, clearly document that its only purpose is to reduce sequences of\n> > write() calls to a single function call, but that the additional writev\n> > guarantees are not needed.\n> \n> It is philosophically more \"pure\" to have a two-level abstraction\n> where write_gather(), which may be inspired by writev(2) but with\n> specific subset of semantics that the application needs, is used by\n> the application and have platforms with good enough writev(2) to\n> implement it in terms of it.  Other platforms may implement it\n> differently, like a series of write(2) calls, and as long as it\n> fulfills the need of write_gather(), we are OK.\n> \n> Doing so would also help in a minuscule way to avoid adding to the\n> complaints we sometimes hear that our internal implementation\n> assumes platform support for POSIX API and semantics way too much\n> even when we do not need to.\n> \n> So I do not mind going in that direction.  It feels a slightly\n> roundabout approach, but in the longer run, I think it would place\n> us in a much better place.\n> \n> I think Patrick's writev(2) follows the pattern our previous compat/\n> routines have taken.  We use real writev(2) where it is available,\n> and in the fake implementations in compat/ we have comments that\n> essentially say \"the real function offers X, Y, and Z, but we only\n> want X and Z and do not need Y, so this implementation does not\n> support Y\".  It is harder to maintain because the application side\n> may be tempted over time to start depending on Y.  If some platforms\n> cannot easily provide an equivalent of the real function, it is\n> easier for them if the rules explicitly state from the beginning\n> that we do not require and will never require Y, needing only X and\n> Z from either the fake or real implementation.\n> \n> At that point, we are not describing the real function anymore, so\n> your proposal to give it a specific name is one step away from that,\n> and that step is in the right direction.\n\nYeah, I was mostly trying to follow the precedent that we currently have\nin our code base, where we assume POSIX functions and paper over any\ngaps that a specific platform has via compatibility wrappers. And I\nthink that the compatibility wrapper we have for writev(3p) is close\nenough to the original semantics of it to not really matter much in\npractice.\n\nI overall don't disagree that it would've been nice at times to have a\nhigher-level interface that abstracts over such platform specifics\nwithout assuming POSIX semantics. But I'm not really sure what it buys\nus to rename this to `write_gather()` without rethinking the bigger\napproach we have to I/O. That is, what does it buy us to now diverge\nfrom the current practice, and where do we want to end up?\n\nI feel like that's a much bigger discussion to be had, and I'm not a\n100% sure whether I want to open that can of worms now. If the only\nthing that I need to change is to rename from writev to write_gather\nthen I'm happy to do that. But as said, I don't really think this buys\nus much without the bigger discussion, so I'm a bit hesitant to do this.\n\nPatrick\n"},{"id":"549746","messageId":"xmqqy0ekr0bm.fsf@gitster.g","threadId":"66012","inReplyTo":"anL0nIk6uzkYR9Oc@pks.im","subject":"Re: [PATCH 0/5] Reintroduce writev(3p)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-05T16:36:45Z","receivedAt":"2026-08-05T16:36:48Z","isPatch":true,"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Mon, Jul 27, 2026 at 08:44:39AM -0700, Junio C Hamano wrote:\n>> Johannes Sixt <j6t@kdbg.org> writes:\n>> \n>> I think Patrick's writev(2) follows the pattern our previous compat/\n>> routines have taken.  We use real writev(2) where it is available,\n>> and in the fake implementations in compat/ we have comments that\n>> essentially say \"the real function offers X, Y, and Z, but we only\n>> want X and Z and do not need Y, so this implementation does not\n>> support Y\".  It is harder to maintain because the application side\n>> may be tempted over time to start depending on Y.  If some platforms\n>> cannot easily provide an equivalent of the real function, it is\n>> easier for them if the rules explicitly state from the beginning\n>> that we do not require and will never require Y, needing only X and\n>> Z from either the fake or real implementation.\n>> \n>> At that point, we are not describing the real function anymore, so\n>> your proposal to give it a specific name is one step away from that,\n>> and that step is in the right direction.\n>\n> Yeah, I was mostly trying to follow the precedent that we currently have\n> in our code base, where we assume POSIX functions and paper over any\n> gaps that a specific platform has via compatibility wrappers. And I\n> think that the compatibility wrapper we have for writev(3p) is close\n> enough to the original semantics of it to not really matter much in\n> practice.\n>\n> I overall don't disagree that it would've been nice at times to have a\n> higher-level interface that abstracts over such platform specifics\n> without assuming POSIX semantics. But I'm not really sure what it buys\n> us to rename this to `write_gather()` without rethinking the bigger\n> approach we have to I/O. That is, what does it buy us to now diverge\n> from the current practice, and where do we want to end up?\n\nAs I am not the party who needs to implement a good enough\nemulation, and what is hard to do in non POSIX environment that is\nneeded for writev(2) emulation, I think that is a question for j6t.\n\nA different way to put the same question is \"what is Y in the\ncontext of the intended uses of writev(2) in our codebase\"?\n\nThanks.\n"},{"id":"549752","messageId":"2952d9a7-c7c0-44c0-a321-7ddad6497a6e@kdbg.org","threadId":"66012","inReplyTo":"xmqqy0ekr0bm.fsf@gitster.g","subject":"Re: [PATCH 0/5] Reintroduce writev(3p)","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2026-08-05T17:55:30Z","receivedAt":"2026-08-05T17:55:45Z","isPatch":true,"body":"Am 05.08.26 um 18:36 schrieb Junio C Hamano:\n>>> I think Patrick's writev(2) follows the pattern our previous compat/\n>>> routines have taken.  We use real writev(2) where it is available,\n>>> and in the fake implementations in compat/ we have comments that\n>>> essentially say \"the real function offers X, Y, and Z, but we only\n>>> want X and Z and do not need Y, so this implementation does not\n>>> support Y\".\n\n> A different way to put the same question is \"what is Y in the\n> context of the intended uses of writev(2) in our codebase\"?\nThe Y that I am thinking of primarily is the atomicity guarantee:\n\n> The  data transfers performed by readv() and writev() are atomic: the\n> data written by writev() is written as a single block that is not\n> intermingled with output from writes in  other  processes; [...]\n\n(See `man 2 writev`; this isn't spelled out explicitly in the Open Group\nBase Specification.)\n\nThis is basically unimplementable by any emulation that has to call\nwrite() multiple times.\n\n-- Hannes\n\n"},{"id":"549765","messageId":"xmqqpkzwpg1g.fsf@gitster.g","threadId":"66012","inReplyTo":"2952d9a7-c7c0-44c0-a321-7ddad6497a6e@kdbg.org","subject":"Re: [PATCH 0/5] Reintroduce writev(3p)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-05T18:40:11Z","receivedAt":"2026-08-05T18:40:15Z","isPatch":true,"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> Am 05.08.26 um 18:36 schrieb Junio C Hamano:\n>>>> I think Patrick's writev(2) follows the pattern our previous compat/\n>>>> routines have taken.  We use real writev(2) where it is available,\n>>>> and in the fake implementations in compat/ we have comments that\n>>>> essentially say \"the real function offers X, Y, and Z, but we only\n>>>> want X and Z and do not need Y, so this implementation does not\n>>>> support Y\".\n>\n>> A different way to put the same question is \"what is Y in the\n>> context of the intended uses of writev(2) in our codebase\"?\n> The Y that I am thinking of primarily is the atomicity guarantee:\n>\n>> The  data transfers performed by readv() and writev() are atomic: the\n>> data written by writev() is written as a single block that is not\n>> intermingled with output from writes in  other  processes; [...]\n>\n> (See `man 2 writev`; this isn't spelled out explicitly in the Open Group\n> Base Specification.)\n>\n> This is basically unimplementable by any emulation that has to call\n> write() multiple times.\n\nLooking at hits from 'git grep -e writev seen', the only two places\nwe use writev() or write_gather() are:\n\n - fast-import, where we write out concatenation of the object\n   header, the payload, and the trailing newline in three separate\n   buffers in one go; and\n\n - sideband, where we send the length and band designator in one\n   buffer and the payload in another in one go.\n\nNeither use would work at all if we had competing writers working in\nparallel to write to the same pipe, regardless of whether atomicity\nis guaranteed.\n\nI think it is OK to explicitly document that any writev(2) emulation\nis allowed to be non-atomic, and it is also OK to declare that using\nwritev(2) in this application to allow competing writes to the same\ndestination is a bug.\n"},{"id":"549774","messageId":"8ff3c3ea-2604-4d65-8a7d-6acd6218b7c2@kdbg.org","threadId":"66012","inReplyTo":"xmqqpkzwpg1g.fsf@gitster.g","subject":"Re: [PATCH 0/5] Reintroduce writev(3p)","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2026-08-05T20:00:51Z","receivedAt":"2026-08-05T20:01:03Z","isPatch":true,"body":"Am 05.08.26 um 20:40 schrieb Junio C Hamano:\n> I think it is OK to explicitly document that any writev(2) emulation\n> is allowed to be non-atomic, and it is also OK to declare that using\n> writev(2) in this application to allow competing writes to the same\n> destination is a bug.\n\nThese are fine.\n\nBut I'm not worried about current uses of writev, I'm worried about\nfuture uses: \"Look, we already use writev elsewhere. Let's use it here,\ntoo, where we can take adavantage of the atomicity of the write.\" It's\ntoo easy to miss a note about non-atomic emulations when the function\nname advertises more than can be guaranteed. For this reason, I strongly\nsuggest to use a different name.\n\n-- Hannes\n\n"},{"id":"549776","messageId":"xmqqa4r0payv.fsf@gitster.g","threadId":"66012","inReplyTo":"8ff3c3ea-2604-4d65-8a7d-6acd6218b7c2@kdbg.org","subject":"Re: [PATCH 0/5] Reintroduce writev(3p)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-05T20:29:44Z","receivedAt":"2026-08-05T20:29:48Z","isPatch":true,"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> Am 05.08.26 um 20:40 schrieb Junio C Hamano:\n>> I think it is OK to explicitly document that any writev(2) emulation\n>> is allowed to be non-atomic, and it is also OK to declare that using\n>> writev(2) in this application to allow competing writes to the same\n>> destination is a bug.\n>\n> These are fine.\n>\n> But I'm not worried about current uses of writev, I'm worried about\n> future uses: \"Look, we already use writev elsewhere. Let's use it here,\n> too, where we can take adavantage of the atomicity of the write.\" It's\n> too easy to miss a note about non-atomic emulations when the function\n> name advertises more than can be guaranteed. For this reason, I strongly\n> suggest to use a different name.\n\nThat is why I added the \"it is also OK to declare\" in the above.\n"},{"id":"549794","messageId":"anQpop92SCAA2C9z@pks.im","threadId":"66012","inReplyTo":"xmqqa4r0payv.fsf@gitster.g","subject":"Re: [PATCH 0/5] Reintroduce writev(3p)","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-06T06:28:50Z","receivedAt":"2026-08-06T06:28:58Z","isPatch":true,"body":"On Wed, Aug 05, 2026 at 01:29:44PM -0700, Junio C Hamano wrote:\n> Johannes Sixt <j6t@kdbg.org> writes:\n> \n> > Am 05.08.26 um 20:40 schrieb Junio C Hamano:\n> >> I think it is OK to explicitly document that any writev(2) emulation\n> >> is allowed to be non-atomic, and it is also OK to declare that using\n> >> writev(2) in this application to allow competing writes to the same\n> >> destination is a bug.\n> >\n> > These are fine.\n> >\n> > But I'm not worried about current uses of writev, I'm worried about\n> > future uses: \"Look, we already use writev elsewhere. Let's use it here,\n> > too, where we can take adavantage of the atomicity of the write.\" It's\n> > too easy to miss a note about non-atomic emulations when the function\n> > name advertises more than can be guaranteed. For this reason, I strongly\n> > suggest to use a different name.\n> \n> That is why I added the \"it is also OK to declare\" in the above.\n\nWe could of course trivially restore the non-interleaving property by\nonly ever writing the first iovec. POSIX doesn't guarantee that the full\niovec is being written, and write(3p) is already non-interleaving. It\nwouldn't even be less efficient compared to the current implementation,\nas we have to loop around write(3p) anyway in our compatibility wrapper.\n\nPatrick\n"},{"id":"549883","messageId":"xmqqcxvvhu6q.fsf@gitster.g","threadId":"66012","inReplyTo":"anQpop92SCAA2C9z@pks.im","subject":"Re: [PATCH 0/5] Reintroduce writev(3p)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-06T20:26:21Z","receivedAt":"2026-08-06T20:26:24Z","isPatch":true,"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Wed, Aug 05, 2026 at 01:29:44PM -0700, Junio C Hamano wrote:\n>> Johannes Sixt <j6t@kdbg.org> writes:\n>> \n>> > Am 05.08.26 um 20:40 schrieb Junio C Hamano:\n>> >> I think it is OK to explicitly document that any writev(2) emulation\n>> >> is allowed to be non-atomic, and it is also OK to declare that using\n>> >> writev(2) in this application to allow competing writes to the same\n>> >> destination is a bug.\n>> >\n>> > These are fine.\n>> >\n>> > But I'm not worried about current uses of writev, I'm worried about\n>> > future uses: \"Look, we already use writev elsewhere. Let's use it here,\n>> > too, where we can take adavantage of the atomicity of the write.\" It's\n>> > too easy to miss a note about non-atomic emulations when the function\n>> > name advertises more than can be guaranteed. For this reason, I strongly\n>> > suggest to use a different name.\n>> \n>> That is why I added the \"it is also OK to declare\" in the above.\n>\n> We could of course trivially restore the non-interleaving property by\n> only ever writing the first iovec. POSIX doesn't guarantee that the full\n> iovec is being written, and write(3p) is already non-interleaving. It\n> wouldn't even be less efficient compared to the current implementation,\n> as we have to loop around write(3p) anyway in our compatibility wrapper.\n\nOK, by castrating the writev(2) emulation implementation to write\nout only the first iovec[], we are making the emulation \"atomic\", so\nthere is no need to say \"your emulation does not have to be atomic\"\nand we can rely on being able to pretend that we have writev(2)\navailable everywhere.  Also, it is a bug on the programmers' side to\nassume that their writev() calls will not result in a short write,\nso it does not have to be spelled out, either, which automatically\nmeans you'd better be calling writev_in_full() and not writev()\nitself.\n\nI can buy that.  Clever.  It means we'd need an update for [PATCH\n1/5] 1ed0bc4e3b (compat/posix: introduce writev(3p) wrapper,\n2026-07-16), right?  The update would be a simplification that loses\na lot of code (and overflow check), which is even nicer ;-).\n\n\n"},{"id":"549926","messageId":"20260807-pks-reintroduce-writev-v2-0-30fcff0e89c1@pks.im","threadId":"66012","inReplyTo":"20260716-pks-reintroduce-writev-v1-0-ea9038c884bc@pks.im","subject":"[PATCH v2 0/5] Reintroduce writev(3p)","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-07T06:18:02Z","receivedAt":"2026-08-07T06:18:14Z","isPatch":true,"body":"Hi,\n\nthis patch series reintroduces the writev(3p) wrapper. This wrapper was\noriginally introduced as part of Git 2.54 [1], but was ejected due to\nissues on NonStop [2].\n\nThis patch series here revives the effort with a couple of fixes on top:\n\n  - It picks Dscho's fix for CMake [3].\n\n  - It picks a fix for NonStop [4] and polishes it a bit.\n\n  - It adapts one more site to demonstrate that its usefulness is not\n    limited to a single callsite, only.\n\nFurthermore, I have included benchmarks now that demonstrate the\nbenefits to make this series a bit more appealing. Ultimately, I'd be\nfine if we say we rather don't want to go this way though. I merely\nwanted to tie some loose ends that I left dangling.\n\nThat, and it's nice to not work on pluggable object databases once in a\nwhile.\n\nChanges in v2:\n  - Adapt the writev compatibility shim to not do torn writes anymore\n    across multiple iovecs. Instead, we now only write the first iovec,\n    which should be fine as callers are expected to loop around writev\n    anyway.\n  - Link to v1: https://patch.msgid.link/20260716-pks-reintroduce-writev-v1-0-ea9038c884bc@pks.im\n\nThanks!\n\nPatrick\n\n[1]: <20260227-pks-upload-pack-write-contention-v1-0-7166fe255704@pks.im>\n[2]: <028901dcc859$d2419470$76c4bd50$@nexbridge.com>\n[3]: <pull.2078.git.1775206502134.gitgitgadget@gmail.com>\n[4]: <20260409-b4-pks-writev-max-io-size-v1-1-81730e8f35df@pks.im>\n\n---\nPatrick Steinhardt (5):\n      compat/posix: introduce writev(3p) wrapper\n      wrapper: introduce writev(3p) wrappers\n      wrapper: properly handle MAX_IO_SIZE in writev(3p)\n      sideband: use writev(3p) to send pktlines\n      fast-import: use writev(3p) to send cat-blob responses\n\n Makefile                            |  4 ++\n builtin/fast-import.c               | 18 +++++++--\n compat/posix.h                      | 14 +++++++\n compat/writev.c                     | 41 +++++++++++++++++++\n config.mak.uname                    |  2 +\n contrib/buildsystems/CMakeLists.txt |  6 ++-\n meson.build                         |  1 +\n sideband.c                          | 14 +++++--\n wrapper.c                           | 78 +++++++++++++++++++++++++++++++++++++\n wrapper.h                           | 10 +++++\n write-or-die.c                      |  8 ++++\n write-or-die.h                      |  1 +\n 12 files changed, 190 insertions(+), 7 deletions(-)\n\nRange-diff versus v1:\n\n1:  69b8be6ec5 ! 1:  f519260452 compat/posix: introduce writev(3p) wrapper\n    @@ compat/writev.c (new)\n     +\n     +ssize_t git_writev(int fd, const struct iovec *iov, int iovcnt)\n     +{\n    -+\tsize_t total_written = 0;\n     +\tsize_t sum = 0;\n     +\n    ++\tif (iovcnt <= 0) {\n    ++\t\terrno = EINVAL;\n    ++\t\treturn -1;\n    ++\t}\n    ++\n     +\t/*\n     +\t * According to writev(3p), the syscall shall error with EINVAL in case\n     +\t * the sum of `iov_len` overflows `ssize_t`.\n     +\t */\n     +\tfor (int i = 0; i < iovcnt; i++) {\n     +\t\tif (iov[i].iov_len > maximum_signed_value_of_type(ssize_t) ||\n    ++\t\t    unsigned_add_overflows(iov[i].iov_len, sum) ||\n     +\t\t    iov[i].iov_len + sum > maximum_signed_value_of_type(ssize_t)) {\n     +\t\t\terrno = EINVAL;\n     +\t\t\treturn -1;\n    @@ compat/writev.c (new)\n     +\t\tsum += iov[i].iov_len;\n     +\t}\n     +\n    ++\t/*\n    ++\t * We only ever write the first non-empty vector so that we can\n    ++\t * guarantee the call to be non-interleaving as guaranteed by POSIX.\n    ++\t * This works just fine as callers have to loop around writev anyway.\n    ++\t */\n     +\tfor (int i = 0; i < iovcnt; i++) {\n    -+\t\tconst char *bytes = iov[i].iov_base;\n    -+\t\tsize_t iovec_written = 0;\n    -+\n    -+\t\twhile (iovec_written < iov[i].iov_len) {\n    -+\t\t\tssize_t bytes_written = xwrite(fd, bytes + iovec_written,\n    -+\t\t\t\t\t\t       iov[i].iov_len - iovec_written);\n    -+\t\t\tif (bytes_written < 0) {\n    -+\t\t\t\tif (total_written)\n    -+\t\t\t\t\tgoto out;\n    -+\t\t\t\treturn bytes_written;\n    -+\t\t\t}\n    -+\t\t\tif (!bytes_written)\n    -+\t\t\t\tgoto out;\n    -+\t\t\tiovec_written += bytes_written;\n    -+\t\t\ttotal_written += bytes_written;\n    -+\t\t}\n    ++\t\tif (!iov[i].iov_len)\n    ++\t\t\tcontinue;\n    ++\t\treturn xwrite(fd, iov[i].iov_base, iov[i].iov_len);\n     +\t}\n     +\n    -+out:\n    -+\treturn (ssize_t) total_written;\n    ++\t/* When all iovec members were zero we ought to return 0 according to POSIX. */\n    ++\treturn 0;\n     +}\n     \n      ## config.mak.uname ##\n2:  f6013a18ba = 2:  41814b6668 wrapper: introduce writev(3p) wrappers\n3:  3dc0eff00b = 3:  af2e351491 wrapper: properly handle MAX_IO_SIZE in writev(3p)\n4:  95c872432f = 4:  d3ba9d73d2 sideband: use writev(3p) to send pktlines\n5:  4c1efb5284 = 5:  148a2c8928 fast-import: use writev(3p) to send cat-blob responses\n\n---\nbase-commit: 55526a18268bbc1ddaf8a6b7850c33d984eac9e9\nchange-id: 20260714-pks-reintroduce-writev-2d8f7e52eee9\n\n"},{"id":"549927","messageId":"20260807-pks-reintroduce-writev-v2-1-30fcff0e89c1@pks.im","threadId":"66012","inReplyTo":"20260807-pks-reintroduce-writev-v2-0-30fcff0e89c1@pks.im","subject":"[PATCH v2 1/5] compat/posix: introduce writev(3p) wrapper","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-07T06:18:03Z","receivedAt":"2026-08-07T06:18:15Z","isPatch":true,"body":"In a subsequent commit we're going to add the first caller to\nwritev(3p). Introduce a compatibility wrapper for this syscall that we\ncan use on systems that don't have this syscall.\n\nThe syscall exists on modern Unixes like Linux and macOS, and seemingly\neven for NonStop according to [1]. It doesn't seem to exist on Windows\nthough.\n\n[1]: http://nonstoptools.com/manuals/OSS-SystemCalls.pdf\n[2]: https://www.gnu.org/software/gnulib/manual/html_node/writev.html\n\nHelped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Makefile                            |  4 ++++\n compat/posix.h                      | 14 +++++++++++++\n compat/writev.c                     | 41 +++++++++++++++++++++++++++++++++++++\n config.mak.uname                    |  2 ++\n contrib/buildsystems/CMakeLists.txt |  6 +++++-\n meson.build                         |  1 +\n 6 files changed, 67 insertions(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex 1f3f099f5c..eda5ecc5b4 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2033,6 +2033,10 @@ ifdef NO_PREAD\n \tCOMPAT_CFLAGS += -DNO_PREAD\n \tCOMPAT_OBJS += compat/pread.o\n endif\n+ifdef NO_WRITEV\n+\tCOMPAT_CFLAGS += -DNO_WRITEV\n+\tCOMPAT_OBJS += compat/writev.o\n+endif\n ifdef NO_FAST_WORKING_DIRECTORY\n \tBASIC_CFLAGS += -DNO_FAST_WORKING_DIRECTORY\n endif\ndiff --git a/compat/posix.h b/compat/posix.h\nindex e2e794cad7..71cc731620 100644\n--- a/compat/posix.h\n+++ b/compat/posix.h\n@@ -148,6 +148,9 @@\n #include <sys/socket.h>\n #include <sys/ioctl.h>\n #include <sys/statvfs.h>\n+#ifndef NO_WRITEV\n+#include <sys/uio.h>\n+#endif\n #include <termios.h>\n #ifndef NO_SYS_SELECT_H\n #include <sys/select.h>\n@@ -334,6 +337,17 @@ int git_lstat(const char *, struct stat *);\n ssize_t git_pread(int fd, void *buf, size_t count, off_t offset);\n #endif\n \n+#ifdef NO_WRITEV\n+#define writev git_writev\n+#define iovec git_iovec\n+struct git_iovec {\n+\tvoid *iov_base;\n+\tsize_t iov_len;\n+};\n+\n+ssize_t git_writev(int fd, const struct iovec *iov, int iovcnt);\n+#endif\n+\n #ifdef NO_SETENV\n #define setenv gitsetenv\n int gitsetenv(const char *, const char *, int);\ndiff --git a/compat/writev.c b/compat/writev.c\nnew file mode 100644\nindex 0000000000..540f66de61\n--- /dev/null\n+++ b/compat/writev.c\n@@ -0,0 +1,41 @@\n+#include \"../git-compat-util.h\"\n+#include \"../wrapper.h\"\n+\n+ssize_t git_writev(int fd, const struct iovec *iov, int iovcnt)\n+{\n+\tsize_t sum = 0;\n+\n+\tif (iovcnt <= 0) {\n+\t\terrno = EINVAL;\n+\t\treturn -1;\n+\t}\n+\n+\t/*\n+\t * According to writev(3p), the syscall shall error with EINVAL in case\n+\t * the sum of `iov_len` overflows `ssize_t`.\n+\t */\n+\tfor (int i = 0; i < iovcnt; i++) {\n+\t\tif (iov[i].iov_len > maximum_signed_value_of_type(ssize_t) ||\n+\t\t    unsigned_add_overflows(iov[i].iov_len, sum) ||\n+\t\t    iov[i].iov_len + sum > maximum_signed_value_of_type(ssize_t)) {\n+\t\t\terrno = EINVAL;\n+\t\t\treturn -1;\n+\t\t}\n+\n+\t\tsum += iov[i].iov_len;\n+\t}\n+\n+\t/*\n+\t * We only ever write the first non-empty vector so that we can\n+\t * guarantee the call to be non-interleaving as guaranteed by POSIX.\n+\t * This works just fine as callers have to loop around writev anyway.\n+\t */\n+\tfor (int i = 0; i < iovcnt; i++) {\n+\t\tif (!iov[i].iov_len)\n+\t\t\tcontinue;\n+\t\treturn xwrite(fd, iov[i].iov_base, iov[i].iov_len);\n+\t}\n+\n+\t/* When all iovec members were zero we ought to return 0 according to POSIX. */\n+\treturn 0;\n+}\ndiff --git a/config.mak.uname b/config.mak.uname\nindex 9ebd240378..95ef6e64dc 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -483,6 +483,7 @@ ifeq ($(uname_S),Windows)\n \tSANE_TOOL_PATH ?= $(msvc_bin_dir_msys)\n \tHAVE_ALLOCA_H = YesPlease\n \tNO_PREAD = YesPlease\n+\tNO_WRITEV = YesPlease\n \tNEEDS_CRYPTO_WITH_SSL = YesPlease\n \tNO_LIBGEN_H = YesPlease\n \tNO_POLL = YesPlease\n@@ -697,6 +698,7 @@ ifeq ($(uname_S),MINGW)\n \tpathsep = ;\n \tHAVE_ALLOCA_H = YesPlease\n \tNO_PREAD = YesPlease\n+\tNO_WRITEV = YesPlease\n \tNEEDS_CRYPTO_WITH_SSL = YesPlease\n \tNO_LIBGEN_H = YesPlease\n \tNO_POLL = YesPlease\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex a57c4b464f..8f56203f34 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -378,7 +378,7 @@ endif()\n #function checks\n set(function_checks\n \tstrcasestr memmem strlcpy strtoimax strtoumax strtoull\n-\tsetenv mkdtemp poll pread memmem)\n+\tsetenv mkdtemp poll pread memmem writev)\n \n #unsetenv,hstrerror are incompatible with windows build\n if(NOT WIN32)\n@@ -423,6 +423,10 @@ if(NOT HAVE_MEMMEM)\n \tlist(APPEND compat_SOURCES compat/memmem.c)\n endif()\n \n+if(NOT HAVE_WRITEV)\n+\tlist(APPEND compat_SOURCES compat/writev.c)\n+endif()\n+\n if(NOT WIN32)\n \tif(NOT HAVE_UNSETENV)\n \t\tlist(APPEND compat_SOURCES compat/unsetenv.c)\ndiff --git a/meson.build b/meson.build\nindex ca235801cf..613828ff25 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -1446,6 +1446,7 @@ checkfuncs = {\n   'initgroups' : [],\n   'strtoumax' : ['strtoumax.c', 'strtoimax.c'],\n   'pread' : ['pread.c'],\n+  'writev' : ['writev.c'],\n }\n \n if host_machine.system() == 'windows'\n\n-- \n2.55.0.679.g6767b8d81c.dirty\n\n"},{"id":"549929","messageId":"20260807-pks-reintroduce-writev-v2-2-30fcff0e89c1@pks.im","threadId":"66012","inReplyTo":"20260807-pks-reintroduce-writev-v2-0-30fcff0e89c1@pks.im","subject":"[PATCH v2 2/5] wrapper: introduce writev(3p) wrappers","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-07T06:18:04Z","receivedAt":"2026-08-07T06:18:18Z","isPatch":true,"body":"In the preceding commit we have added a compatibility wrapper for the\nwritev(3p) syscall. Introduce some generic wrappers for this function\nthat we nowadays take for granted in the Git codebase.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n wrapper.c      | 41 +++++++++++++++++++++++++++++++++++++++++\n wrapper.h      |  9 +++++++++\n write-or-die.c |  8 ++++++++\n write-or-die.h |  1 +\n 4 files changed, 59 insertions(+)\n\ndiff --git a/wrapper.c b/wrapper.c\nindex 16f5a63fbb..be8fa575e6 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -323,6 +323,47 @@ ssize_t write_in_full(int fd, const void *buf, size_t count)\n \treturn total;\n }\n \n+ssize_t writev_in_full(int fd, struct iovec *iov, int iovcnt)\n+{\n+\tssize_t total_written = 0;\n+\n+\twhile (iovcnt) {\n+\t\tssize_t bytes_written = writev(fd, iov, iovcnt);\n+\t\tif (bytes_written < 0) {\n+\t\t\tif (errno == EINTR || errno == EAGAIN)\n+\t\t\t\tcontinue;\n+\t\t\treturn -1;\n+\t\t}\n+\t\tif (!bytes_written) {\n+\t\t\terrno = ENOSPC;\n+\t\t\treturn -1;\n+\t\t}\n+\n+\t\ttotal_written += bytes_written;\n+\n+\t\t/*\n+\t\t * We first need to discard any iovec entities that have been\n+\t\t * fully written.\n+\t\t */\n+\t\twhile (iovcnt && (size_t)bytes_written >= iov->iov_len) {\n+\t\t\tbytes_written -= iov->iov_len;\n+\t\t\tiov++;\n+\t\t\tiovcnt--;\n+\t\t}\n+\n+\t\t/*\n+\t\t * Finally, we need to adjust the last iovec in case we have\n+\t\t * performed a partial write.\n+\t\t */\n+\t\tif (iovcnt && bytes_written) {\n+\t\t\tiov->iov_base = (char *) iov->iov_base + bytes_written;\n+\t\t\tiov->iov_len -= bytes_written;\n+\t\t}\n+\t}\n+\n+\treturn total_written;\n+}\n+\n ssize_t pread_in_full(int fd, void *buf, size_t count, off_t offset)\n {\n \tchar *p = buf;\ndiff --git a/wrapper.h b/wrapper.h\nindex 15ac3bab6e..27519b32d1 100644\n--- a/wrapper.h\n+++ b/wrapper.h\n@@ -47,6 +47,15 @@ ssize_t read_in_full(int fd, void *buf, size_t count);\n ssize_t write_in_full(int fd, const void *buf, size_t count);\n ssize_t pread_in_full(int fd, void *buf, size_t count, off_t offset);\n \n+/*\n+ * Try to write all iovecs. Returns -1 in case an error occurred with a proper\n+ * errno set, the number of bytes written otherwise.\n+ *\n+ * Note that the iovec will be modified as a result of this call to adjust for\n+ * partial writes!\n+ */\n+ssize_t writev_in_full(int fd, struct iovec *iov, int iovcnt);\n+\n static inline ssize_t write_str_in_full(int fd, const char *str)\n {\n \treturn write_in_full(fd, str, strlen(str));\ndiff --git a/write-or-die.c b/write-or-die.c\nindex 01a9a51fa2..5f522fb728 100644\n--- a/write-or-die.c\n+++ b/write-or-die.c\n@@ -96,6 +96,14 @@ void write_or_die(int fd, const void *buf, size_t count)\n \t}\n }\n \n+void writev_or_die(int fd, struct iovec *iov, int iovlen)\n+{\n+\tif (writev_in_full(fd, iov, iovlen) < 0) {\n+\t\tcheck_pipe(errno);\n+\t\tdie_errno(\"writev error\");\n+\t}\n+}\n+\n void fwrite_or_die(FILE *f, const void *buf, size_t count)\n {\n \tif (fwrite(buf, 1, count, f) != count)\ndiff --git a/write-or-die.h b/write-or-die.h\nindex ff0408bd84..a045bdfaef 100644\n--- a/write-or-die.h\n+++ b/write-or-die.h\n@@ -7,6 +7,7 @@ void fprintf_or_die(FILE *, const char *fmt, ...);\n void fwrite_or_die(FILE *f, const void *buf, size_t count);\n void fflush_or_die(FILE *f);\n void write_or_die(int fd, const void *buf, size_t count);\n+void writev_or_die(int fd, struct iovec *iov, int iovlen);\n \n /*\n  * These values are used to help identify parts of a repository to fsync.\n\n-- \n2.55.0.679.g6767b8d81c.dirty\n\n"},{"id":"549930","messageId":"20260807-pks-reintroduce-writev-v2-3-30fcff0e89c1@pks.im","threadId":"66012","inReplyTo":"20260807-pks-reintroduce-writev-v2-0-30fcff0e89c1@pks.im","subject":"[PATCH v2 3/5] wrapper: properly handle MAX_IO_SIZE in writev(3p)","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-07T06:18:05Z","receivedAt":"2026-08-07T06:18:20Z","isPatch":true,"body":"Some systems like NonStop set a comparatively small `MAX_IO_SIZE`, which\nlimits the maximum number of bytes we're allowed to write in a single\ncall. We already handle this limit properly in `xwrite()`, but we have\nrecently introduced wrappers for writev(3p) where we don't. This will\ncause the syscall to return EINVAL in case somebody passes an iovec\nentry to writev(3p) that is larger than `MAX_IO_SIZE`.\n\nIntroduce a new function `xwritev()` that is similar to `xwrite()` in\nthat it handles such platform-specific nuances:\n\n  - We only pass the leading iovec entries to writev(3p) that fit into\n    `MAX_IO_SIZE`, pretending that the underlying syscall performed a\n    short write. This mirrors how `xwrite()` chomps overly large\n    requests before handing them to write(3p). As a consequence, callers\n    will never see writev(3p)'s EINVAL error for requests whose summed\n    length would overflow an ssize_t, but observe a short write instead.\n\n  - If already the first iovec entry exceeds the limit we instead punt\n    to `xwrite()`, which knows to handle this case for us.\n\n  - We restart the underlying syscall on EINTR and EAGAIN, just like\n    `xwrite()` does for write(3p).\n\nAdapt `writev_in_full()` to use this new wrapper. With the retry logic\nnow living in `xwritev()`, the calling loop becomes the exact mirror\nimage of `write_in_full()`, which also retains the responsibility of\ntranslating a zero-length write into ENOSPC.\n\nReported-by: Randall Becker <randall.becker@nexbridge.ca>\nHelped-by: Jeff King <peff@peff.net>\nHelped-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n wrapper.c | 47 ++++++++++++++++++++++++++++++++++++++++++-----\n wrapper.h |  1 +\n 2 files changed, 43 insertions(+), 5 deletions(-)\n\ndiff --git a/wrapper.c b/wrapper.c\nindex be8fa575e6..561f9ee9c9 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -323,17 +323,54 @@ ssize_t write_in_full(int fd, const void *buf, size_t count)\n \treturn total;\n }\n \n+ssize_t xwritev(int fd, struct iovec *iov, int iovcnt)\n+{\n+\tsize_t allowed = MAX_IO_SIZE;\n+\tint i;\n+\n+\t/*\n+\t * Some platforms define a comparatively small `MAX_IO_SIZE` that\n+\t * limits how many bytes can be written with a single call to\n+\t * write(3p) or writev(3p); exceeding that limit causes the syscall to\n+\t * fail with EINVAL. Just like xwrite() chomps overly large requests\n+\t * for write(3p), pretend that the underlying writev(3p) performed a\n+\t * short write by only passing along the leading iovec entries that\n+\t * fit into that limit.\n+\t */\n+\tfor (i = 0; i < iovcnt; i++) {\n+\t\tif (iov[i].iov_len > allowed) {\n+\t\t\t/*\n+\t\t\t * If the first buffer is larger than MAX_IO_SIZE,\n+\t\t\t * let xwrite() deal with it.\n+\t\t\t */\n+\t\t\tif (!i)\n+\t\t\t\treturn xwrite(fd, iov->iov_base, iov->iov_len);\n+\t\t\tbreak;\n+\t\t}\n+\t\tallowed -= iov[i].iov_len;\n+\t}\n+\n+\twhile (1) {\n+\t\tssize_t bytes_written = writev(fd, iov, i);\n+\t\tif (bytes_written < 0) {\n+\t\t\tif (errno == EINTR)\n+\t\t\t\tcontinue;\n+\t\t\tif (handle_nonblock(fd, POLLOUT, errno))\n+\t\t\t\tcontinue;\n+\t\t}\n+\n+\t\treturn bytes_written;\n+\t}\n+}\n+\n ssize_t writev_in_full(int fd, struct iovec *iov, int iovcnt)\n {\n \tssize_t total_written = 0;\n \n \twhile (iovcnt) {\n-\t\tssize_t bytes_written = writev(fd, iov, iovcnt);\n-\t\tif (bytes_written < 0) {\n-\t\t\tif (errno == EINTR || errno == EAGAIN)\n-\t\t\t\tcontinue;\n+\t\tssize_t bytes_written = xwritev(fd, iov, iovcnt);\n+\t\tif (bytes_written < 0)\n \t\t\treturn -1;\n-\t\t}\n \t\tif (!bytes_written) {\n \t\t\terrno = ENOSPC;\n \t\t\treturn -1;\ndiff --git a/wrapper.h b/wrapper.h\nindex 27519b32d1..a6287d7f4d 100644\n--- a/wrapper.h\n+++ b/wrapper.h\n@@ -16,6 +16,7 @@ void *xmmap_gently(void *start, size_t length, int prot, int flags, int fd, off_\n int xopen(const char *path, int flags, ...);\n ssize_t xread(int fd, void *buf, size_t len);\n ssize_t xwrite(int fd, const void *buf, size_t len);\n+ssize_t xwritev(int fd, struct iovec *iov, int iovcnt);\n ssize_t xpread(int fd, void *buf, size_t len, off_t offset);\n int xdup(int fd);\n FILE *xfopen(const char *path, const char *mode);\n\n-- \n2.55.0.679.g6767b8d81c.dirty\n\n"},{"id":"549931","messageId":"20260807-pks-reintroduce-writev-v2-4-30fcff0e89c1@pks.im","threadId":"66012","inReplyTo":"20260807-pks-reintroduce-writev-v2-0-30fcff0e89c1@pks.im","subject":"[PATCH v2 4/5] sideband: use writev(3p) to send pktlines","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-07T06:18:06Z","receivedAt":"2026-08-07T06:18:24Z","isPatch":true,"body":"Every pktline that we send out via `send_sideband()` currently requires\ntwo syscalls: one to write the pktline's length, and one to send its\ndata. This typically isn't all that much of a problem, but under extreme\nload the syscalls may cause contention in the kernel.\n\nRefactor the code to instead use the newly introduced writev(3p) infra\nso that we can send out the data with a single syscall. This reduces the\nnumber of syscalls from around 133,000 calls to write(3p) to around\n67,000 calls to writev(3p).\n\nThis change leads to a performance improvement for git-upload-pack(1),\nbut we have to cheat a bit to really make it measurable. Usually, the\ntime is strongly dominated by generating the packfile itself. But if we\nprecompute the pack and serve it via the pack-objects hook then we can\nessentially eliminate that overhead. The following setup is executed in\nthe Git repository:\n\n  $ cat >request <<-EOF\n  0048want 5ce91c059e41090e7d2cffad39c04af8acf98dc1 side-band no-progress\n  00000009done\n  EOF\n  $ echo 5ce91c059e41090e7d2cffad39c04af8acf98dc1 | git pack-objects --revs --stdout >pack\n  $ cat >hook <<-EOF\n  #!/bin/sh\n  cat >/dev/null\n  cat \"$(pwd)\"/pack\n  EOF\n  $ chmod u+x hook\n  $ git -c uploadpack.packObjectsHook=\"$(pwd)\"/hook upload-pack . <request\n\nBenchmarking the last command leads to the following results:\n\n  Benchmark 1: HEAD~\n    Time (mean ± σ):     192.9 ms ±   0.6 ms    [User: 106.5 ms, System: 95.3 ms]\n    Range (min … max):   191.7 ms … 194.1 ms    50 runs\n\n  Benchmark 2: HEAD\n    Time (mean ± σ):     141.1 ms ±   0.7 ms    [User: 63.2 ms, System: 86.6 ms]\n    Range (min … max):   139.8 ms … 142.7 ms    50 runs\n\n  Summary\n    HEAD ran\n      1.37 ± 0.01 times faster than HEAD~\n\nThis might not be impressive in absolute numbers when you also take into\naccount the time it takes to generate the packfile itself. But GitLab\n(and supposedly other forges) have caching mechanisms in place that work\nexactly like the above setup, where repeated incoming requests can be\nserved from the same cached packfile. And in those cases, the impact is\nsizeable.\n\nMore importantly though, as hinted at above, GitLab has observed in the\npast that with enough cache hits we eventually start to saturate a\nsemaphore in the Linux kernel itself in the pipe write path. This\nbottleneck is being moved a bit by having to do less syscalls.\n\nSuggested-by: Jeff King <peff@peff.net>\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n sideband.c | 14 +++++++++++---\n 1 file changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git a/sideband.c b/sideband.c\nindex 1523a53e1d..94e5b56172 100644\n--- a/sideband.c\n+++ b/sideband.c\n@@ -441,6 +441,7 @@ void send_sideband(int fd, int band, const char *data, ssize_t sz, int packet_ma\n \tconst char *p = data;\n \n \twhile (sz) {\n+\t\tstruct iovec iov[2];\n \t\tunsigned n;\n \t\tchar hdr[5];\n \n@@ -450,12 +451,19 @@ void send_sideband(int fd, int band, const char *data, ssize_t sz, int packet_ma\n \t\tif (0 <= band) {\n \t\t\txsnprintf(hdr, sizeof(hdr), \"%04x\", n + 5);\n \t\t\thdr[4] = band;\n-\t\t\twrite_or_die(fd, hdr, 5);\n+\t\t\tiov[0].iov_base = hdr;\n+\t\t\tiov[0].iov_len = 5;\n \t\t} else {\n \t\t\txsnprintf(hdr, sizeof(hdr), \"%04x\", n + 4);\n-\t\t\twrite_or_die(fd, hdr, 4);\n+\t\t\tiov[0].iov_base = hdr;\n+\t\t\tiov[0].iov_len = 4;\n \t\t}\n-\t\twrite_or_die(fd, p, n);\n+\n+\t\tiov[1].iov_base = (void *) p;\n+\t\tiov[1].iov_len = n;\n+\n+\t\twritev_or_die(fd, iov, ARRAY_SIZE(iov));\n+\n \t\tp += n;\n \t\tsz -= n;\n \t}\n\n-- \n2.55.0.679.g6767b8d81c.dirty\n\n"},{"id":"549932","messageId":"20260807-pks-reintroduce-writev-v2-5-30fcff0e89c1@pks.im","threadId":"66012","inReplyTo":"20260807-pks-reintroduce-writev-v2-0-30fcff0e89c1@pks.im","subject":"[PATCH v2 5/5] fast-import: use writev(3p) to send cat-blob responses","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-07T06:18:07Z","receivedAt":"2026-08-07T06:18:26Z","isPatch":true,"body":"When answering a `cat-blob` command, `cat_blob()` issues three separate\ncalls to write(3p) on the cat-blob fd: one for the header line, one for\nthe full blob payload, and one for the trailing newline. Frontends like\ngit-filter-repo issue these commands in bulk, once per rewritten blob,\nso the syscall overhead adds up.\n\nUse `writev_in_full()` to send all three parts with a single syscall.\n\nThis can be benchmarked with the following setup:\n\n    $ git cat-file --unordered --filter=object:type=blob\n        --batch-check='cat-blob %(objectname)' --batch-all-objects >request\n    $ git fast-import --cat-blob-fd=3 <request\n\nExecuting this with 100,000 objects in linux.git:\n\n  Benchmark 1: HEAD~\n    Time (mean ± σ):      1.320 s ±  0.003 s    [User: 1.154 s, System: 0.161 s]\n    Range (min … max):    1.314 s …  1.324 s    10 runs\n\n  Benchmark 2: HEAD\n    Time (mean ± σ):      1.270 s ±  0.022 s    [User: 1.133 s, System: 0.132 s]\n    Range (min … max):    1.209 s …  1.282 s    10 runs\n\n  Summary\n    HEAD ran\n      1.04 ± 0.02 times faster than HEAD~\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n builtin/fast-import.c | 18 +++++++++++++++---\n 1 file changed, 15 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/fast-import.c b/builtin/fast-import.c\nindex aa656c5195..48fda01c94 100644\n--- a/builtin/fast-import.c\n+++ b/builtin/fast-import.c\n@@ -3332,6 +3332,7 @@ static void cat_blob_write(const char *buf, unsigned long size)\n static void cat_blob(struct object_entry *oe, struct object_id *oid)\n {\n \tstruct strbuf line = STRBUF_INIT;\n+\tstruct iovec iov[3];\n \tunsigned long size;\n \tenum object_type type = 0;\n \tchar *buf;\n@@ -3365,10 +3366,21 @@ static void cat_blob(struct object_entry *oe, struct object_id *oid)\n \tstrbuf_reset(&line);\n \tstrbuf_addf(&line, \"%s %s %\"PRIuMAX\"\\n\", oid_to_hex(oid),\n \t\t    type_name(type), (uintmax_t)size);\n-\tcat_blob_write(line.buf, line.len);\n+\n+\t/*\n+\t * Write the header, the payload and the trailing newline with a\n+\t * single writev(3p) call instead of three separate write(3p) calls.\n+\t */\n+\tiov[0].iov_base = line.buf;\n+\tiov[0].iov_len = line.len;\n+\tiov[1].iov_base = buf;\n+\tiov[1].iov_len = size;\n+\tiov[2].iov_base = (void *) \"\\n\";\n+\tiov[2].iov_len = 1;\n+\n+\tif (writev_in_full(cat_blob_fd, iov, ARRAY_SIZE(iov)) < 0)\n+\t\tdie_errno(_(\"write to frontend failed\"));\n \tstrbuf_release(&line);\n-\tcat_blob_write(buf, size);\n-\tcat_blob_write(\"\\n\", 1);\n \tif (oe && oe->pack_id == pack_id) {\n \t\tlast_blob.offset = oe->idx.offset;\n \t\tstrbuf_attach(&last_blob.data, buf, size, size + 1);\n\n-- \n2.55.0.679.g6767b8d81c.dirty\n\n"},{"id":"549933","messageId":"anV7Z1fmC-HhaNfy@pks.im","threadId":"66012","inReplyTo":"xmqqcxvvhu6q.fsf@gitster.g","subject":"Re: [PATCH 0/5] Reintroduce writev(3p)","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-07T06:29:59Z","receivedAt":"2026-08-07T06:30:07Z","isPatch":true,"body":"On Thu, Aug 06, 2026 at 01:26:21PM -0700, Junio C Hamano wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > On Wed, Aug 05, 2026 at 01:29:44PM -0700, Junio C Hamano wrote:\n> >> Johannes Sixt <j6t@kdbg.org> writes:\n> >> \n> >> > Am 05.08.26 um 20:40 schrieb Junio C Hamano:\n> >> >> I think it is OK to explicitly document that any writev(2) emulation\n> >> >> is allowed to be non-atomic, and it is also OK to declare that using\n> >> >> writev(2) in this application to allow competing writes to the same\n> >> >> destination is a bug.\n> >> >\n> >> > These are fine.\n> >> >\n> >> > But I'm not worried about current uses of writev, I'm worried about\n> >> > future uses: \"Look, we already use writev elsewhere. Let's use it here,\n> >> > too, where we can take adavantage of the atomicity of the write.\" It's\n> >> > too easy to miss a note about non-atomic emulations when the function\n> >> > name advertises more than can be guaranteed. For this reason, I strongly\n> >> > suggest to use a different name.\n> >> \n> >> That is why I added the \"it is also OK to declare\" in the above.\n> >\n> > We could of course trivially restore the non-interleaving property by\n> > only ever writing the first iovec. POSIX doesn't guarantee that the full\n> > iovec is being written, and write(3p) is already non-interleaving. It\n> > wouldn't even be less efficient compared to the current implementation,\n> > as we have to loop around write(3p) anyway in our compatibility wrapper.\n> \n> OK, by castrating the writev(2) emulation implementation to write\n> out only the first iovec[], we are making the emulation \"atomic\", so\n> there is no need to say \"your emulation does not have to be atomic\"\n> and we can rely on being able to pretend that we have writev(2)\n> available everywhere.  Also, it is a bug on the programmers' side to\n> assume that their writev() calls will not result in a short write,\n> so it does not have to be spelled out, either, which automatically\n> means you'd better be calling writev_in_full() and not writev()\n> itself.\n> \n> I can buy that.  Clever.  It means we'd need an update for [PATCH\n> 1/5] 1ed0bc4e3b (compat/posix: introduce writev(3p) wrapper,\n> 2026-07-16), right?  The update would be a simplification that loses\n> a lot of code (and overflow check), which is even nicer ;-).\n\nYeah, exactly. The overflow check I think we should keep though to be\ncloser to the POSIX requirements.\n\nPatrick\n"}]}