[PATCH 0/5] Reintroduce writev(3p)

[PATCH 0/5] Reintroduce writev(3p)

[Patrick Steinhardt]

Hi,

this patch series reintroduces the writev(3p) wrapper. This wrapper was
originally introduced as part of Git 2.54 [1], but was ejected due to
issues on NonStop [2].

This patch series here revives the effort with a couple of fixes on top:

  - It picks Dscho's fix for CMake [3].

  - It picks a fix for NonStop [4] and polishes it a bit.

  - It adapts one more site to demonstrate that its usefulness is not
    limited to a single callsite, only.

Furthermore, I have included benchmarks now that demonstrate the
benefits to make this series a bit more appealing. Ultimately, I'd be
fine if we say we rather don't want to go this way though. I merely
wanted to tie some loose ends that I left dangling.

That, and it's nice to not work on pluggable object databases once in a
while.

Thanks!

Patrick

[1]: <20260227-pks-upload-pack-write-contention-v1-0-7166fe255704@pks.im>
[2]: <028901dcc859$d2419470$76c4bd50$@nexbridge.com>
[3]: <pull.2078.git.1775206502134.gitgitgadget@gmail.com>
[4]: <20260409-b4-pks-writev-max-io-size-v1-1-81730e8f35df@pks.im>

---
Patrick Steinhardt (5):
      compat/posix: introduce writev(3p) wrapper
      wrapper: introduce writev(3p) wrappers
      wrapper: properly handle MAX_IO_SIZE in writev(3p)
      sideband: use writev(3p) to send pktlines
      fast-import: use writev(3p) to send cat-blob responses

 Makefile                            |  4 ++
 builtin/fast-import.c               | 18 +++++++--
 compat/posix.h                      | 14 +++++++
 compat/writev.c                     | 44 +++++++++++++++++++++
 config.mak.uname                    |  2 +
 contrib/buildsystems/CMakeLists.txt |  6 ++-
 meson.build                         |  1 +
 sideband.c                          | 14 +++++--
 wrapper.c                           | 78 +++++++++++++++++++++++++++++++++++++
 wrapper.h                           | 10 +++++
 write-or-die.c                      |  8 ++++
 write-or-die.h                      |  1 +
 12 files changed, 193 insertions(+), 7 deletions(-)


---
base-commit: 55526a18268bbc1ddaf8a6b7850c33d984eac9e9
change-id: 20260714-pks-reintroduce-writev-2d8f7e52eee9

[PATCH 1/5] compat/posix: introduce writev(3p) wrapper

[Patrick Steinhardt]

In a subsequent commit we're going to add the first caller to
writev(3p). Introduce a compatibility wrapper for this syscall that we
can use on systems that don't have this syscall.

The syscall exists on modern Unixes like Linux and macOS, and seemingly
even for NonStop according to [1]. It doesn't seem to exist on Windows
though.

[1]: http://nonstoptools.com/manuals/OSS-SystemCalls.pdf
[2]: https://www.gnu.org/software/gnulib/manual/html_node/writev.html

Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Makefile                            |  4 ++++
 compat/posix.h                      | 14 ++++++++++++
 compat/writev.c                     | 44 +++++++++++++++++++++++++++++++++++++
 config.mak.uname                    |  2 ++
 contrib/buildsystems/CMakeLists.txt |  6 ++++-
 meson.build                         |  1 +
 6 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 1f3f099f5c..eda5ecc5b4 100644
--- a/Makefile
+++ b/Makefile
@@ -2033,6 +2033,10 @@ ifdef NO_PREAD
 	COMPAT_CFLAGS += -DNO_PREAD
 	COMPAT_OBJS += compat/pread.o
 endif
+ifdef NO_WRITEV
+	COMPAT_CFLAGS += -DNO_WRITEV
+	COMPAT_OBJS += compat/writev.o
+endif
 ifdef NO_FAST_WORKING_DIRECTORY
 	BASIC_CFLAGS += -DNO_FAST_WORKING_DIRECTORY
 endif
diff --git a/compat/posix.h b/compat/posix.h
index e2e794cad7..71cc731620 100644
--- a/compat/posix.h
+++ b/compat/posix.h
@@ -148,6 +148,9 @@
 #include <sys/socket.h>
 #include <sys/ioctl.h>
 #include <sys/statvfs.h>
+#ifndef NO_WRITEV
+#include <sys/uio.h>
+#endif
 #include <termios.h>
 #ifndef NO_SYS_SELECT_H
 #include <sys/select.h>
@@ -334,6 +337,17 @@ int git_lstat(const char *, struct stat *);
 ssize_t git_pread(int fd, void *buf, size_t count, off_t offset);
 #endif
 
+#ifdef NO_WRITEV
+#define writev git_writev
+#define iovec git_iovec
+struct git_iovec {
+	void *iov_base;
+	size_t iov_len;
+};
+
+ssize_t git_writev(int fd, const struct iovec *iov, int iovcnt);
+#endif
+
 #ifdef NO_SETENV
 #define setenv gitsetenv
 int gitsetenv(const char *, const char *, int);
diff --git a/compat/writev.c b/compat/writev.c
new file mode 100644
index 0000000000..ab2e223634
--- /dev/null
+++ b/compat/writev.c
@@ -0,0 +1,44 @@
+#include "../git-compat-util.h"
+#include "../wrapper.h"
+
+ssize_t git_writev(int fd, const struct iovec *iov, int iovcnt)
+{
+	size_t total_written = 0;
+	size_t sum = 0;
+
+	/*
+	 * According to writev(3p), the syscall shall error with EINVAL in case
+	 * the sum of `iov_len` overflows `ssize_t`.
+	 */
+	for (int i = 0; i < iovcnt; i++) {
+		if (iov[i].iov_len > maximum_signed_value_of_type(ssize_t) ||
+		    iov[i].iov_len + sum > maximum_signed_value_of_type(ssize_t)) {
+			errno = EINVAL;
+			return -1;
+		}
+
+		sum += iov[i].iov_len;
+	}
+
+	for (int i = 0; i < iovcnt; i++) {
+		const char *bytes = iov[i].iov_base;
+		size_t iovec_written = 0;
+
+		while (iovec_written < iov[i].iov_len) {
+			ssize_t bytes_written = xwrite(fd, bytes + iovec_written,
+						       iov[i].iov_len - iovec_written);
+			if (bytes_written < 0) {
+				if (total_written)
+					goto out;
+				return bytes_written;
+			}
+			if (!bytes_written)
+				goto out;
+			iovec_written += bytes_written;
+			total_written += bytes_written;
+		}
+	}
+
+out:
+	return (ssize_t) total_written;
+}
diff --git a/config.mak.uname b/config.mak.uname
index 9ebd240378..95ef6e64dc 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -483,6 +483,7 @@ ifeq ($(uname_S),Windows)
 	SANE_TOOL_PATH ?= $(msvc_bin_dir_msys)
 	HAVE_ALLOCA_H = YesPlease
 	NO_PREAD = YesPlease
+	NO_WRITEV = YesPlease
 	NEEDS_CRYPTO_WITH_SSL = YesPlease
 	NO_LIBGEN_H = YesPlease
 	NO_POLL = YesPlease
@@ -697,6 +698,7 @@ ifeq ($(uname_S),MINGW)
 	pathsep = ;
 	HAVE_ALLOCA_H = YesPlease
 	NO_PREAD = YesPlease
+	NO_WRITEV = YesPlease
 	NEEDS_CRYPTO_WITH_SSL = YesPlease
 	NO_LIBGEN_H = YesPlease
 	NO_POLL = YesPlease
diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index a57c4b464f..8f56203f34 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -378,7 +378,7 @@ endif()
 #function checks
 set(function_checks
 	strcasestr memmem strlcpy strtoimax strtoumax strtoull
-	setenv mkdtemp poll pread memmem)
+	setenv mkdtemp poll pread memmem writev)
 
 #unsetenv,hstrerror are incompatible with windows build
 if(NOT WIN32)
@@ -423,6 +423,10 @@ if(NOT HAVE_MEMMEM)
 	list(APPEND compat_SOURCES compat/memmem.c)
 endif()
 
+if(NOT HAVE_WRITEV)
+	list(APPEND compat_SOURCES compat/writev.c)
+endif()
+
 if(NOT WIN32)
 	if(NOT HAVE_UNSETENV)
 		list(APPEND compat_SOURCES compat/unsetenv.c)
diff --git a/meson.build b/meson.build
index ca235801cf..613828ff25 100644
--- a/meson.build
+++ b/meson.build
@@ -1446,6 +1446,7 @@ checkfuncs = {
   'initgroups' : [],
   'strtoumax' : ['strtoumax.c', 'strtoimax.c'],
   'pread' : ['pread.c'],
+  'writev' : ['writev.c'],
 }
 
 if host_machine.system() == 'windows'

-- 
2.55.0.313.g8d093f411d.dirty

[PATCH 2/5] wrapper: introduce writev(3p) wrappers

[Patrick Steinhardt]

In the preceding commit we have added a compatibility wrapper for the
writev(3p) syscall. Introduce some generic wrappers for this function
that we nowadays take for granted in the Git codebase.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 wrapper.c      | 41 +++++++++++++++++++++++++++++++++++++++++
 wrapper.h      |  9 +++++++++
 write-or-die.c |  8 ++++++++
 write-or-die.h |  1 +
 4 files changed, 59 insertions(+)

diff --git a/wrapper.c b/wrapper.c
index 16f5a63fbb..be8fa575e6 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -323,6 +323,47 @@ ssize_t write_in_full(int fd, const void *buf, size_t count)
 	return total;
 }
 
+ssize_t writev_in_full(int fd, struct iovec *iov, int iovcnt)
+{
+	ssize_t total_written = 0;
+
+	while (iovcnt) {
+		ssize_t bytes_written = writev(fd, iov, iovcnt);
+		if (bytes_written < 0) {
+			if (errno == EINTR || errno == EAGAIN)
+				continue;
+			return -1;
+		}
+		if (!bytes_written) {
+			errno = ENOSPC;
+			return -1;
+		}
+
+		total_written += bytes_written;
+
+		/*
+		 * We first need to discard any iovec entities that have been
+		 * fully written.
+		 */
+		while (iovcnt && (size_t)bytes_written >= iov->iov_len) {
+			bytes_written -= iov->iov_len;
+			iov++;
+			iovcnt--;
+		}
+
+		/*
+		 * Finally, we need to adjust the last iovec in case we have
+		 * performed a partial write.
+		 */
+		if (iovcnt && bytes_written) {
+			iov->iov_base = (char *) iov->iov_base + bytes_written;
+			iov->iov_len -= bytes_written;
+		}
+	}
+
+	return total_written;
+}
+
 ssize_t pread_in_full(int fd, void *buf, size_t count, off_t offset)
 {
 	char *p = buf;
diff --git a/wrapper.h b/wrapper.h
index 15ac3bab6e..27519b32d1 100644
--- a/wrapper.h
+++ b/wrapper.h
@@ -47,6 +47,15 @@ ssize_t read_in_full(int fd, void *buf, size_t count);
 ssize_t write_in_full(int fd, const void *buf, size_t count);
 ssize_t pread_in_full(int fd, void *buf, size_t count, off_t offset);
 
+/*
+ * Try to write all iovecs. Returns -1 in case an error occurred with a proper
+ * errno set, the number of bytes written otherwise.
+ *
+ * Note that the iovec will be modified as a result of this call to adjust for
+ * partial writes!
+ */
+ssize_t writev_in_full(int fd, struct iovec *iov, int iovcnt);
+
 static inline ssize_t write_str_in_full(int fd, const char *str)
 {
 	return write_in_full(fd, str, strlen(str));
diff --git a/write-or-die.c b/write-or-die.c
index 01a9a51fa2..5f522fb728 100644
--- a/write-or-die.c
+++ b/write-or-die.c
@@ -96,6 +96,14 @@ void write_or_die(int fd, const void *buf, size_t count)
 	}
 }
 
+void writev_or_die(int fd, struct iovec *iov, int iovlen)
+{
+	if (writev_in_full(fd, iov, iovlen) < 0) {
+		check_pipe(errno);
+		die_errno("writev error");
+	}
+}
+
 void fwrite_or_die(FILE *f, const void *buf, size_t count)
 {
 	if (fwrite(buf, 1, count, f) != count)
diff --git a/write-or-die.h b/write-or-die.h
index ff0408bd84..a045bdfaef 100644
--- a/write-or-die.h
+++ b/write-or-die.h
@@ -7,6 +7,7 @@ void fprintf_or_die(FILE *, const char *fmt, ...);
 void fwrite_or_die(FILE *f, const void *buf, size_t count);
 void fflush_or_die(FILE *f);
 void write_or_die(int fd, const void *buf, size_t count);
+void writev_or_die(int fd, struct iovec *iov, int iovlen);
 
 /*
  * These values are used to help identify parts of a repository to fsync.

-- 
2.55.0.313.g8d093f411d.dirty

[PATCH 3/5] wrapper: properly handle MAX_IO_SIZE in writev(3p)

[Patrick Steinhardt]

Some systems like NonStop set a comparatively small `MAX_IO_SIZE`, which
limits the maximum number of bytes we're allowed to write in a single
call. We already handle this limit properly in `xwrite()`, but we have
recently introduced wrappers for writev(3p) where we don't. This will
cause the syscall to return EINVAL in case somebody passes an iovec
entry to writev(3p) that is larger than `MAX_IO_SIZE`.

Introduce a new function `xwritev()` that is similar to `xwrite()` in
that it handles such platform-specific nuances:

  - We only pass the leading iovec entries to writev(3p) that fit into
    `MAX_IO_SIZE`, pretending that the underlying syscall performed a
    short write. This mirrors how `xwrite()` chomps overly large
    requests before handing them to write(3p). As a consequence, callers
    will never see writev(3p)'s EINVAL error for requests whose summed
    length would overflow an ssize_t, but observe a short write instead.

  - If already the first iovec entry exceeds the limit we instead punt
    to `xwrite()`, which knows to handle this case for us.

  - We restart the underlying syscall on EINTR and EAGAIN, just like
    `xwrite()` does for write(3p).

Adapt `writev_in_full()` to use this new wrapper. With the retry logic
now living in `xwritev()`, the calling loop becomes the exact mirror
image of `write_in_full()`, which also retains the responsibility of
translating a zero-length write into ENOSPC.

Reported-by: Randall Becker <randall.becker@nexbridge.ca>
Helped-by: Jeff King <peff@peff.net>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 wrapper.c | 47 ++++++++++++++++++++++++++++++++++++++++++-----
 wrapper.h |  1 +
 2 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/wrapper.c b/wrapper.c
index be8fa575e6..561f9ee9c9 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -323,17 +323,54 @@ ssize_t write_in_full(int fd, const void *buf, size_t count)
 	return total;
 }
 
+ssize_t xwritev(int fd, struct iovec *iov, int iovcnt)
+{
+	size_t allowed = MAX_IO_SIZE;
+	int i;
+
+	/*
+	 * Some platforms define a comparatively small `MAX_IO_SIZE` that
+	 * limits how many bytes can be written with a single call to
+	 * write(3p) or writev(3p); exceeding that limit causes the syscall to
+	 * fail with EINVAL. Just like xwrite() chomps overly large requests
+	 * for write(3p), pretend that the underlying writev(3p) performed a
+	 * short write by only passing along the leading iovec entries that
+	 * fit into that limit.
+	 */
+	for (i = 0; i < iovcnt; i++) {
+		if (iov[i].iov_len > allowed) {
+			/*
+			 * If the first buffer is larger than MAX_IO_SIZE,
+			 * let xwrite() deal with it.
+			 */
+			if (!i)
+				return xwrite(fd, iov->iov_base, iov->iov_len);
+			break;
+		}
+		allowed -= iov[i].iov_len;
+	}
+
+	while (1) {
+		ssize_t bytes_written = writev(fd, iov, i);
+		if (bytes_written < 0) {
+			if (errno == EINTR)
+				continue;
+			if (handle_nonblock(fd, POLLOUT, errno))
+				continue;
+		}
+
+		return bytes_written;
+	}
+}
+
 ssize_t writev_in_full(int fd, struct iovec *iov, int iovcnt)
 {
 	ssize_t total_written = 0;
 
 	while (iovcnt) {
-		ssize_t bytes_written = writev(fd, iov, iovcnt);
-		if (bytes_written < 0) {
-			if (errno == EINTR || errno == EAGAIN)
-				continue;
+		ssize_t bytes_written = xwritev(fd, iov, iovcnt);
+		if (bytes_written < 0)
 			return -1;
-		}
 		if (!bytes_written) {
 			errno = ENOSPC;
 			return -1;
diff --git a/wrapper.h b/wrapper.h
index 27519b32d1..a6287d7f4d 100644
--- a/wrapper.h
+++ b/wrapper.h
@@ -16,6 +16,7 @@ void *xmmap_gently(void *start, size_t length, int prot, int flags, int fd, off_
 int xopen(const char *path, int flags, ...);
 ssize_t xread(int fd, void *buf, size_t len);
 ssize_t xwrite(int fd, const void *buf, size_t len);
+ssize_t xwritev(int fd, struct iovec *iov, int iovcnt);
 ssize_t xpread(int fd, void *buf, size_t len, off_t offset);
 int xdup(int fd);
 FILE *xfopen(const char *path, const char *mode);

-- 
2.55.0.313.g8d093f411d.dirty

[PATCH 4/5] sideband: use writev(3p) to send pktlines

[Patrick Steinhardt]

Every pktline that we send out via `send_sideband()` currently requires
two syscalls: one to write the pktline's length, and one to send its
data. This typically isn't all that much of a problem, but under extreme
load the syscalls may cause contention in the kernel.

Refactor the code to instead use the newly introduced writev(3p) infra
so that we can send out the data with a single syscall. This reduces the
number of syscalls from around 133,000 calls to write(3p) to around
67,000 calls to writev(3p).

This change leads to a performance improvement for git-upload-pack(1),
but we have to cheat a bit to really make it measurable. Usually, the
time is strongly dominated by generating the packfile itself. But if we
precompute the pack and serve it via the pack-objects hook then we can
essentially eliminate that overhead. The following setup is executed in
the Git repository:

  $ cat >request <<-EOF
  0048want 5ce91c059e41090e7d2cffad39c04af8acf98dc1 side-band no-progress
  00000009done
  EOF
  $ echo 5ce91c059e41090e7d2cffad39c04af8acf98dc1 | git pack-objects --revs --stdout >pack
  $ cat >hook <<-EOF
  #!/bin/sh
  cat >/dev/null
  cat "$(pwd)"/pack
  EOF
  $ chmod u+x hook
  $ git -c uploadpack.packObjectsHook="$(pwd)"/hook upload-pack . <request

Benchmarking the last command leads to the following results:

  Benchmark 1: HEAD~
    Time (mean ± σ):     192.9 ms ±   0.6 ms    [User: 106.5 ms, System: 95.3 ms]
    Range (min … max):   191.7 ms … 194.1 ms    50 runs

  Benchmark 2: HEAD
    Time (mean ± σ):     141.1 ms ±   0.7 ms    [User: 63.2 ms, System: 86.6 ms]
    Range (min … max):   139.8 ms … 142.7 ms    50 runs

  Summary
    HEAD ran
      1.37 ± 0.01 times faster than HEAD~

This might not be impressive in absolute numbers when you also take into
account the time it takes to generate the packfile itself. But GitLab
(and supposedly other forges) have caching mechanisms in place that work
exactly like the above setup, where repeated incoming requests can be
served from the same cached packfile. And in those cases, the impact is
sizeable.

More importantly though, as hinted at above, GitLab has observed in the
past that with enough cache hits we eventually start to saturate a
semaphore in the Linux kernel itself in the pipe write path. This
bottleneck is being moved a bit by having to do less syscalls.

Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 sideband.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/sideband.c b/sideband.c
index 1523a53e1d..94e5b56172 100644
--- a/sideband.c
+++ b/sideband.c
@@ -441,6 +441,7 @@ void send_sideband(int fd, int band, const char *data, ssize_t sz, int packet_ma
 	const char *p = data;
 
 	while (sz) {
+		struct iovec iov[2];
 		unsigned n;
 		char hdr[5];
 
@@ -450,12 +451,19 @@ void send_sideband(int fd, int band, const char *data, ssize_t sz, int packet_ma
 		if (0 <= band) {
 			xsnprintf(hdr, sizeof(hdr), "%04x", n + 5);
 			hdr[4] = band;
-			write_or_die(fd, hdr, 5);
+			iov[0].iov_base = hdr;
+			iov[0].iov_len = 5;
 		} else {
 			xsnprintf(hdr, sizeof(hdr), "%04x", n + 4);
-			write_or_die(fd, hdr, 4);
+			iov[0].iov_base = hdr;
+			iov[0].iov_len = 4;
 		}
-		write_or_die(fd, p, n);
+
+		iov[1].iov_base = (void *) p;
+		iov[1].iov_len = n;
+
+		writev_or_die(fd, iov, ARRAY_SIZE(iov));
+
 		p += n;
 		sz -= n;
 	}

-- 
2.55.0.313.g8d093f411d.dirty

[PATCH 5/5] fast-import: use writev(3p) to send cat-blob responses

[Patrick Steinhardt]

When answering a `cat-blob` command, `cat_blob()` issues three separate
calls to write(3p) on the cat-blob fd: one for the header line, one for
the full blob payload, and one for the trailing newline. Frontends like
git-filter-repo issue these commands in bulk, once per rewritten blob,
so the syscall overhead adds up.

Use `writev_in_full()` to send all three parts with a single syscall.

This can be benchmarked with the following setup:

    $ git cat-file --unordered --filter=object:type=blob
        --batch-check='cat-blob %(objectname)' --batch-all-objects >request
    $ git fast-import --cat-blob-fd=3 <request

Executing this with 100,000 objects in linux.git:

  Benchmark 1: HEAD~
    Time (mean ± σ):      1.320 s ±  0.003 s    [User: 1.154 s, System: 0.161 s]
    Range (min … max):    1.314 s …  1.324 s    10 runs

  Benchmark 2: HEAD
    Time (mean ± σ):      1.270 s ±  0.022 s    [User: 1.133 s, System: 0.132 s]
    Range (min … max):    1.209 s …  1.282 s    10 runs

  Summary
    HEAD ran
      1.04 ± 0.02 times faster than HEAD~

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/fast-import.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index aa656c5195..48fda01c94 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -3332,6 +3332,7 @@ static void cat_blob_write(const char *buf, unsigned long size)
 static void cat_blob(struct object_entry *oe, struct object_id *oid)
 {
 	struct strbuf line = STRBUF_INIT;
+	struct iovec iov[3];
 	unsigned long size;
 	enum object_type type = 0;
 	char *buf;
@@ -3365,10 +3366,21 @@ static void cat_blob(struct object_entry *oe, struct object_id *oid)
 	strbuf_reset(&line);
 	strbuf_addf(&line, "%s %s %"PRIuMAX"\n", oid_to_hex(oid),
 		    type_name(type), (uintmax_t)size);
-	cat_blob_write(line.buf, line.len);
+
+	/*
+	 * Write the header, the payload and the trailing newline with a
+	 * single writev(3p) call instead of three separate write(3p) calls.
+	 */
+	iov[0].iov_base = line.buf;
+	iov[0].iov_len = line.len;
+	iov[1].iov_base = buf;
+	iov[1].iov_len = size;
+	iov[2].iov_base = (void *) "\n";
+	iov[2].iov_len = 1;
+
+	if (writev_in_full(cat_blob_fd, iov, ARRAY_SIZE(iov)) < 0)
+		die_errno(_("write to frontend failed"));
 	strbuf_release(&line);
-	cat_blob_write(buf, size);
-	cat_blob_write("\n", 1);
 	if (oe && oe->pack_id == pack_id) {
 		last_blob.offset = oe->idx.offset;
 		strbuf_attach(&last_blob.data, buf, size, size + 1);

-- 
2.55.0.313.g8d093f411d.dirty

Re: [PATCH 1/5] compat/posix: introduce writev(3p) wrapper

[Simon Richter]

Hi,

> +		if (iov[i].iov_len > maximum_signed_value_of_type(ssize_t) ||
> +		    iov[i].iov_len + sum > maximum_signed_value_of_type(ssize_t)) {

That feels like it could overflow.

    Simon

Re: [PATCH 0/5] Reintroduce writev(3p)

[Johannes Sixt]

Am 16.07.26 um 09:52 schrieb Patrick Steinhardt:
> this patch series reintroduces the writev(3p) wrapper. This wrapper was
> originally introduced as part of Git 2.54 [1], but was ejected due to
> issues on NonStop [2].

Please don't call the function "writev" so that nobody associates it
with the guarantees that only POSIX provides, but none of the
emulations. Call it "write_gather", for example.

Also, clearly document that its only purpose is to reduce sequences of
write() calls to a single function call, but that the additional writev
guarantees are not needed.

A range-diff to the earlier round would have been very helpful.

-- Hannes

Re: [PATCH 1/5] compat/posix: introduce writev(3p) wrapper

[Junio C Hamano]

Simon Richter <Simon.Richter@hogyros.de> writes:

> Hi,
>
>> +		if (iov[i].iov_len > maximum_signed_value_of_type(ssize_t) ||
>> +		    iov[i].iov_len + sum > maximum_signed_value_of_type(ssize_t)) {
>
> That feels like it could overflow.

Isn't it checking if it would overflow (and dying if so)?

Ah, wait.  The addition "(iov[i].iov_len + sum)" can indeed wrap
around, and comparing it with the maximum value of ssize_t wouldn't
catch that.  Is that what you mean?

Would something like this:

    if (maximum_signed_value_of_type(ssize_t) < iov[i].iov_len ||
	iov[i].iov_len + sum < iov[i].iov_len ||
	maximum_signed_value_of_type(ssize_t) < iov[i].iov_len + sum)

work better to catch the three cases independently?

 (1) The value is already too large on its own.
 (2) Adding them together would cause an unsigned wrap-around.
 (3) The sum does not wrap around, but it exceeds the maximum
     representable value of ssize_t anyway.

Re: [PATCH 1/5] compat/posix: introduce writev(3p) wrapper

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Simon Richter <Simon.Richter@hogyros.de> writes:
>
>> Hi,
>>
>>> +		if (iov[i].iov_len > maximum_signed_value_of_type(ssize_t) ||
>>> +		    iov[i].iov_len + sum > maximum_signed_value_of_type(ssize_t)) {
>>
>> That feels like it could overflow.
>
> Isn't it checking if it would overflow (and dying if so)?
>
> Ah, wait.  The addition "(iov[i].iov_len + sum)" can indeed wrap
> around, and comparing it with the maximum value of ssize_t wouldn't
> catch that.  Is that what you mean?
>
> Would something like this:
>
>     if (maximum_signed_value_of_type(ssize_t) < iov[i].iov_len ||
> 	iov[i].iov_len + sum < iov[i].iov_len ||
> 	maximum_signed_value_of_type(ssize_t) < iov[i].iov_len + sum)
>
> work better to catch the three cases independently?
>
>  (1) The value is already too large on its own.
>  (2) Adding them together would cause an unsigned wrap-around.
>  (3) The sum does not wrap around, but it exceeds the maximum
>      representable value of ssize_t anyway.

Actually, looking at it again, I think the original code is safe
after all, because:

 * "sum", even though it is a size_t, is checked inside the loop to
   ensure it stays below the maximum value of ssize_t each time it
   gets a new value.
 * iov[i].iov_len is checked to ensure it does not exceed the
   maximum value of ssize_t by the first part of the condition.

If both values are less than or equal to the maximum value of
ssize_t, their sum is at most twice that limit.  For an N-bit
size_t, this sum is at most (2^N - 2), which can be computed safely
without any unsigned wrap-around.

So...?

Re: [PATCH 0/5] Reintroduce writev(3p)

[Junio C Hamano]

Johannes Sixt <j6t@kdbg.org> writes:

> Am 16.07.26 um 09:52 schrieb Patrick Steinhardt:
>> this patch series reintroduces the writev(3p) wrapper. This wrapper was
>> originally introduced as part of Git 2.54 [1], but was ejected due to
>> issues on NonStop [2].
>
> Please don't call the function "writev" so that nobody associates it
> with the guarantees that only POSIX provides, but none of the
> emulations. Call it "write_gather", for example.
>
> Also, clearly document that its only purpose is to reduce sequences of
> write() calls to a single function call, but that the additional writev
> guarantees are not needed.

It is philosophically more "pure" to have a two-level abstraction
where write_gather(), which may be inspired by writev(2) but with
specific subset of semantics that the application needs, is used by
the application and have platforms with good enough writev(2) to
implement it in terms of it.  Other platforms may implement it
differently, like a series of write(2) calls, and as long as it
fulfills the need of write_gather(), we are OK.

Doing so would also help in a minuscule way to avoid adding to the
complaints we sometimes hear that our internal implementation
assumes platform support for POSIX API and semantics way too much
even when we do not need to.

So I do not mind going in that direction.  It feels a slightly
roundabout approach, but in the longer run, I think it would place
us in a much better place.

I think Patrick's writev(2) follows the pattern our previous compat/
routines have taken.  We use real writev(2) where it is available,
and in the fake implementations in compat/ we have comments that
essentially say "the real function offers X, Y, and Z, but we only
want X and Z and do not need Y, so this implementation does not
support Y".  It is harder to maintain because the application side
may be tempted over time to start depending on Y.  If some platforms
cannot easily provide an equivalent of the real function, it is
easier for them if the rules explicitly state from the beginning
that we do not require and will never require Y, needing only X and
Z from either the fake or real implementation.

At that point, we are not describing the real function anymore, so
your proposal to give it a specific name is one step away from that,
and that step is in the right direction.

Thanks.

PS.  I was going over the list of "waiting for response" topics, and
this was one of them.  I suspect Patrick and the GitLab team are
still away at an offsite [*], so this is in no way poking him for an
immediate reroll, but rather a note sent while my attention is on
these stalled topics.

https://lore.kernel.org/git/amLgMqkqxR8mKIbT@pks.im/

Re: [PATCH 1/5] compat/posix: introduce writev(3p) wrapper

[Patrick Steinhardt]

On Thu, Jul 16, 2026 at 01:44:18PM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Simon Richter <Simon.Richter@hogyros.de> writes:
> >
> >> Hi,
> >>
> >>> +		if (iov[i].iov_len > maximum_signed_value_of_type(ssize_t) ||
> >>> +		    iov[i].iov_len + sum > maximum_signed_value_of_type(ssize_t)) {
> >>
> >> That feels like it could overflow.
> >
> > Isn't it checking if it would overflow (and dying if so)?
> >
> > Ah, wait.  The addition "(iov[i].iov_len + sum)" can indeed wrap
> > around, and comparing it with the maximum value of ssize_t wouldn't
> > catch that.  Is that what you mean?
> >
> > Would something like this:
> >
> >     if (maximum_signed_value_of_type(ssize_t) < iov[i].iov_len ||
> > 	iov[i].iov_len + sum < iov[i].iov_len ||
> > 	maximum_signed_value_of_type(ssize_t) < iov[i].iov_len + sum)
> >
> > work better to catch the three cases independently?
> >
> >  (1) The value is already too large on its own.
> >  (2) Adding them together would cause an unsigned wrap-around.
> >  (3) The sum does not wrap around, but it exceeds the maximum
> >      representable value of ssize_t anyway.
> 
> Actually, looking at it again, I think the original code is safe
> after all, because:
> 
>  * "sum", even though it is a size_t, is checked inside the loop to
>    ensure it stays below the maximum value of ssize_t each time it
>    gets a new value.
>  * iov[i].iov_len is checked to ensure it does not exceed the
>    maximum value of ssize_t by the first part of the condition.
> 
> If both values are less than or equal to the maximum value of
> ssize_t, their sum is at most twice that limit.  For an N-bit
> size_t, this sum is at most (2^N - 2), which can be computed safely
> without any unsigned wrap-around.
> 
> So...?

Yeah, I think your analysis is correct. It's quite subtle though, so
maybe we should make this a bit more explicit? Something like the
following patch for example:

diff --git a/compat/writev.c b/compat/writev.c
index ab2e223634..960673861d 100644
--- a/compat/writev.c
+++ b/compat/writev.c
@@ -12,6 +12,7 @@ ssize_t git_writev(int fd, const struct iovec *iov, int iovcnt)
 	 */
 	for (int i = 0; i < iovcnt; i++) {
 		if (iov[i].iov_len > maximum_signed_value_of_type(ssize_t) ||
+		    unsigned_add_overflows(iov[i].iov_len, sum) ||
 		    iov[i].iov_len + sum > maximum_signed_value_of_type(ssize_t)) {
 			errno = EINVAL;
 			return -1;

I doubt the performance overhead of this additional check is really
going to matter :)

Patrick

Re: [PATCH 0/5] Reintroduce writev(3p)

[Patrick Steinhardt]

On Mon, Jul 27, 2026 at 08:44:39AM -0700, Junio C Hamano wrote:
> Johannes Sixt <j6t@kdbg.org> writes:
> 
> > Am 16.07.26 um 09:52 schrieb Patrick Steinhardt:
> >> this patch series reintroduces the writev(3p) wrapper. This wrapper was
> >> originally introduced as part of Git 2.54 [1], but was ejected due to
> >> issues on NonStop [2].
> >
> > Please don't call the function "writev" so that nobody associates it
> > with the guarantees that only POSIX provides, but none of the
> > emulations. Call it "write_gather", for example.
> >
> > Also, clearly document that its only purpose is to reduce sequences of
> > write() calls to a single function call, but that the additional writev
> > guarantees are not needed.
> 
> It is philosophically more "pure" to have a two-level abstraction
> where write_gather(), which may be inspired by writev(2) but with
> specific subset of semantics that the application needs, is used by
> the application and have platforms with good enough writev(2) to
> implement it in terms of it.  Other platforms may implement it
> differently, like a series of write(2) calls, and as long as it
> fulfills the need of write_gather(), we are OK.
> 
> Doing so would also help in a minuscule way to avoid adding to the
> complaints we sometimes hear that our internal implementation
> assumes platform support for POSIX API and semantics way too much
> even when we do not need to.
> 
> So I do not mind going in that direction.  It feels a slightly
> roundabout approach, but in the longer run, I think it would place
> us in a much better place.
> 
> I think Patrick's writev(2) follows the pattern our previous compat/
> routines have taken.  We use real writev(2) where it is available,
> and in the fake implementations in compat/ we have comments that
> essentially say "the real function offers X, Y, and Z, but we only
> want X and Z and do not need Y, so this implementation does not
> support Y".  It is harder to maintain because the application side
> may be tempted over time to start depending on Y.  If some platforms
> cannot easily provide an equivalent of the real function, it is
> easier for them if the rules explicitly state from the beginning
> that we do not require and will never require Y, needing only X and
> Z from either the fake or real implementation.
> 
> At that point, we are not describing the real function anymore, so
> your proposal to give it a specific name is one step away from that,
> and that step is in the right direction.

Yeah, I was mostly trying to follow the precedent that we currently have
in our code base, where we assume POSIX functions and paper over any
gaps that a specific platform has via compatibility wrappers. And I
think that the compatibility wrapper we have for writev(3p) is close
enough to the original semantics of it to not really matter much in
practice.

I overall don't disagree that it would've been nice at times to have a
higher-level interface that abstracts over such platform specifics
without assuming POSIX semantics. But I'm not really sure what it buys
us to rename this to `write_gather()` without rethinking the bigger
approach we have to I/O. That is, what does it buy us to now diverge
from the current practice, and where do we want to end up?

I feel like that's a much bigger discussion to be had, and I'm not a
100% sure whether I want to open that can of worms now. If the only
thing that I need to change is to rename from writev to write_gather
then I'm happy to do that. But as said, I don't really think this buys
us much without the bigger discussion, so I'm a bit hesitant to do this.

Patrick

Re: [PATCH 0/5] Reintroduce writev(3p)

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> On Mon, Jul 27, 2026 at 08:44:39AM -0700, Junio C Hamano wrote:
>> Johannes Sixt <j6t@kdbg.org> writes:
>> 
>> I think Patrick's writev(2) follows the pattern our previous compat/
>> routines have taken.  We use real writev(2) where it is available,
>> and in the fake implementations in compat/ we have comments that
>> essentially say "the real function offers X, Y, and Z, but we only
>> want X and Z and do not need Y, so this implementation does not
>> support Y".  It is harder to maintain because the application side
>> may be tempted over time to start depending on Y.  If some platforms
>> cannot easily provide an equivalent of the real function, it is
>> easier for them if the rules explicitly state from the beginning
>> that we do not require and will never require Y, needing only X and
>> Z from either the fake or real implementation.
>> 
>> At that point, we are not describing the real function anymore, so
>> your proposal to give it a specific name is one step away from that,
>> and that step is in the right direction.
>
> Yeah, I was mostly trying to follow the precedent that we currently have
> in our code base, where we assume POSIX functions and paper over any
> gaps that a specific platform has via compatibility wrappers. And I
> think that the compatibility wrapper we have for writev(3p) is close
> enough to the original semantics of it to not really matter much in
> practice.
>
> I overall don't disagree that it would've been nice at times to have a
> higher-level interface that abstracts over such platform specifics
> without assuming POSIX semantics. But I'm not really sure what it buys
> us to rename this to `write_gather()` without rethinking the bigger
> approach we have to I/O. That is, what does it buy us to now diverge
> from the current practice, and where do we want to end up?

As I am not the party who needs to implement a good enough
emulation, and what is hard to do in non POSIX environment that is
needed for writev(2) emulation, I think that is a question for j6t.

A different way to put the same question is "what is Y in the
context of the intended uses of writev(2) in our codebase"?

Thanks.

Re: [PATCH 0/5] Reintroduce writev(3p)

[Johannes Sixt]

Am 05.08.26 um 18:36 schrieb Junio C Hamano:
>>> I think Patrick's writev(2) follows the pattern our previous compat/
>>> routines have taken.  We use real writev(2) where it is available,
>>> and in the fake implementations in compat/ we have comments that
>>> essentially say "the real function offers X, Y, and Z, but we only
>>> want X and Z and do not need Y, so this implementation does not
>>> support Y".

> A different way to put the same question is "what is Y in the
> context of the intended uses of writev(2) in our codebase"?
The Y that I am thinking of primarily is the atomicity guarantee:

> The  data transfers performed by readv() and writev() are atomic: the
> data written by writev() is written as a single block that is not
> intermingled with output from writes in  other  processes; [...]

(See `man 2 writev`; this isn't spelled out explicitly in the Open Group
Base Specification.)

This is basically unimplementable by any emulation that has to call
write() multiple times.

-- Hannes

Re: [PATCH 0/5] Reintroduce writev(3p)

[Junio C Hamano]

Johannes Sixt <j6t@kdbg.org> writes:

> Am 05.08.26 um 18:36 schrieb Junio C Hamano:
>>>> I think Patrick's writev(2) follows the pattern our previous compat/
>>>> routines have taken.  We use real writev(2) where it is available,
>>>> and in the fake implementations in compat/ we have comments that
>>>> essentially say "the real function offers X, Y, and Z, but we only
>>>> want X and Z and do not need Y, so this implementation does not
>>>> support Y".
>
>> A different way to put the same question is "what is Y in the
>> context of the intended uses of writev(2) in our codebase"?
> The Y that I am thinking of primarily is the atomicity guarantee:
>
>> The  data transfers performed by readv() and writev() are atomic: the
>> data written by writev() is written as a single block that is not
>> intermingled with output from writes in  other  processes; [...]
>
> (See `man 2 writev`; this isn't spelled out explicitly in the Open Group
> Base Specification.)
>
> This is basically unimplementable by any emulation that has to call
> write() multiple times.

Looking at hits from 'git grep -e writev seen', the only two places
we use writev() or write_gather() are:

 - fast-import, where we write out concatenation of the object
   header, the payload, and the trailing newline in three separate
   buffers in one go; and

 - sideband, where we send the length and band designator in one
   buffer and the payload in another in one go.

Neither use would work at all if we had competing writers working in
parallel to write to the same pipe, regardless of whether atomicity
is guaranteed.

I think it is OK to explicitly document that any writev(2) emulation
is allowed to be non-atomic, and it is also OK to declare that using
writev(2) in this application to allow competing writes to the same
destination is a bug.

Re: [PATCH 0/5] Reintroduce writev(3p)

[Johannes Sixt]

Am 05.08.26 um 20:40 schrieb Junio C Hamano:
> I think it is OK to explicitly document that any writev(2) emulation
> is allowed to be non-atomic, and it is also OK to declare that using
> writev(2) in this application to allow competing writes to the same
> destination is a bug.

These are fine.

But I'm not worried about current uses of writev, I'm worried about
future uses: "Look, we already use writev elsewhere. Let's use it here,
too, where we can take adavantage of the atomicity of the write." It's
too easy to miss a note about non-atomic emulations when the function
name advertises more than can be guaranteed. For this reason, I strongly
suggest to use a different name.

-- Hannes

Re: [PATCH 0/5] Reintroduce writev(3p)

[Junio C Hamano]

Johannes Sixt <j6t@kdbg.org> writes:

> Am 05.08.26 um 20:40 schrieb Junio C Hamano:
>> I think it is OK to explicitly document that any writev(2) emulation
>> is allowed to be non-atomic, and it is also OK to declare that using
>> writev(2) in this application to allow competing writes to the same
>> destination is a bug.
>
> These are fine.
>
> But I'm not worried about current uses of writev, I'm worried about
> future uses: "Look, we already use writev elsewhere. Let's use it here,
> too, where we can take adavantage of the atomicity of the write." It's
> too easy to miss a note about non-atomic emulations when the function
> name advertises more than can be guaranteed. For this reason, I strongly
> suggest to use a different name.

That is why I added the "it is also OK to declare" in the above.

Re: [PATCH 0/5] Reintroduce writev(3p)

[Patrick Steinhardt]

On Wed, Aug 05, 2026 at 01:29:44PM -0700, Junio C Hamano wrote:
> Johannes Sixt <j6t@kdbg.org> writes:
> 
> > Am 05.08.26 um 20:40 schrieb Junio C Hamano:
> >> I think it is OK to explicitly document that any writev(2) emulation
> >> is allowed to be non-atomic, and it is also OK to declare that using
> >> writev(2) in this application to allow competing writes to the same
> >> destination is a bug.
> >
> > These are fine.
> >
> > But I'm not worried about current uses of writev, I'm worried about
> > future uses: "Look, we already use writev elsewhere. Let's use it here,
> > too, where we can take adavantage of the atomicity of the write." It's
> > too easy to miss a note about non-atomic emulations when the function
> > name advertises more than can be guaranteed. For this reason, I strongly
> > suggest to use a different name.
> 
> That is why I added the "it is also OK to declare" in the above.

We could of course trivially restore the non-interleaving property by
only ever writing the first iovec. POSIX doesn't guarantee that the full
iovec is being written, and write(3p) is already non-interleaving. It
wouldn't even be less efficient compared to the current implementation,
as we have to loop around write(3p) anyway in our compatibility wrapper.

Patrick

Re: [PATCH 0/5] Reintroduce writev(3p)

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> On Wed, Aug 05, 2026 at 01:29:44PM -0700, Junio C Hamano wrote:
>> Johannes Sixt <j6t@kdbg.org> writes:
>> 
>> > Am 05.08.26 um 20:40 schrieb Junio C Hamano:
>> >> I think it is OK to explicitly document that any writev(2) emulation
>> >> is allowed to be non-atomic, and it is also OK to declare that using
>> >> writev(2) in this application to allow competing writes to the same
>> >> destination is a bug.
>> >
>> > These are fine.
>> >
>> > But I'm not worried about current uses of writev, I'm worried about
>> > future uses: "Look, we already use writev elsewhere. Let's use it here,
>> > too, where we can take adavantage of the atomicity of the write." It's
>> > too easy to miss a note about non-atomic emulations when the function
>> > name advertises more than can be guaranteed. For this reason, I strongly
>> > suggest to use a different name.
>> 
>> That is why I added the "it is also OK to declare" in the above.
>
> We could of course trivially restore the non-interleaving property by
> only ever writing the first iovec. POSIX doesn't guarantee that the full
> iovec is being written, and write(3p) is already non-interleaving. It
> wouldn't even be less efficient compared to the current implementation,
> as we have to loop around write(3p) anyway in our compatibility wrapper.

OK, by castrating the writev(2) emulation implementation to write
out only the first iovec[], we are making the emulation "atomic", so
there is no need to say "your emulation does not have to be atomic"
and we can rely on being able to pretend that we have writev(2)
available everywhere.  Also, it is a bug on the programmers' side to
assume that their writev() calls will not result in a short write,
so it does not have to be spelled out, either, which automatically
means you'd better be calling writev_in_full() and not writev()
itself.

I can buy that.  Clever.  It means we'd need an update for [PATCH
1/5] 1ed0bc4e3b (compat/posix: introduce writev(3p) wrapper,
2026-07-16), right?  The update would be a simplification that loses
a lot of code (and overflow check), which is even nicer ;-).

[PATCH v2 0/5] Reintroduce writev(3p)

[Patrick Steinhardt]

Hi,

this patch series reintroduces the writev(3p) wrapper. This wrapper was
originally introduced as part of Git 2.54 [1], but was ejected due to
issues on NonStop [2].

This patch series here revives the effort with a couple of fixes on top:

  - It picks Dscho's fix for CMake [3].

  - It picks a fix for NonStop [4] and polishes it a bit.

  - It adapts one more site to demonstrate that its usefulness is not
    limited to a single callsite, only.

Furthermore, I have included benchmarks now that demonstrate the
benefits to make this series a bit more appealing. Ultimately, I'd be
fine if we say we rather don't want to go this way though. I merely
wanted to tie some loose ends that I left dangling.

That, and it's nice to not work on pluggable object databases once in a
while.

Changes in v2:
  - Adapt the writev compatibility shim to not do torn writes anymore
    across multiple iovecs. Instead, we now only write the first iovec,
    which should be fine as callers are expected to loop around writev
    anyway.
  - Link to v1: https://patch.msgid.link/20260716-pks-reintroduce-writev-v1-0-ea9038c884bc@pks.im

Thanks!

Patrick

[1]: <20260227-pks-upload-pack-write-contention-v1-0-7166fe255704@pks.im>
[2]: <028901dcc859$d2419470$76c4bd50$@nexbridge.com>
[3]: <pull.2078.git.1775206502134.gitgitgadget@gmail.com>
[4]: <20260409-b4-pks-writev-max-io-size-v1-1-81730e8f35df@pks.im>

---
Patrick Steinhardt (5):
      compat/posix: introduce writev(3p) wrapper
      wrapper: introduce writev(3p) wrappers
      wrapper: properly handle MAX_IO_SIZE in writev(3p)
      sideband: use writev(3p) to send pktlines
      fast-import: use writev(3p) to send cat-blob responses

 Makefile                            |  4 ++
 builtin/fast-import.c               | 18 +++++++--
 compat/posix.h                      | 14 +++++++
 compat/writev.c                     | 41 +++++++++++++++++++
 config.mak.uname                    |  2 +
 contrib/buildsystems/CMakeLists.txt |  6 ++-
 meson.build                         |  1 +
 sideband.c                          | 14 +++++--
 wrapper.c                           | 78 +++++++++++++++++++++++++++++++++++++
 wrapper.h                           | 10 +++++
 write-or-die.c                      |  8 ++++
 write-or-die.h                      |  1 +
 12 files changed, 190 insertions(+), 7 deletions(-)

Range-diff versus v1:

1:  69b8be6ec5 ! 1:  f519260452 compat/posix: introduce writev(3p) wrapper
    @@ compat/writev.c (new)
     +
     +ssize_t git_writev(int fd, const struct iovec *iov, int iovcnt)
     +{
    -+	size_t total_written = 0;
     +	size_t sum = 0;
     +
    ++	if (iovcnt <= 0) {
    ++		errno = EINVAL;
    ++		return -1;
    ++	}
    ++
     +	/*
     +	 * According to writev(3p), the syscall shall error with EINVAL in case
     +	 * the sum of `iov_len` overflows `ssize_t`.
     +	 */
     +	for (int i = 0; i < iovcnt; i++) {
     +		if (iov[i].iov_len > maximum_signed_value_of_type(ssize_t) ||
    ++		    unsigned_add_overflows(iov[i].iov_len, sum) ||
     +		    iov[i].iov_len + sum > maximum_signed_value_of_type(ssize_t)) {
     +			errno = EINVAL;
     +			return -1;
    @@ compat/writev.c (new)
     +		sum += iov[i].iov_len;
     +	}
     +
    ++	/*
    ++	 * We only ever write the first non-empty vector so that we can
    ++	 * guarantee the call to be non-interleaving as guaranteed by POSIX.
    ++	 * This works just fine as callers have to loop around writev anyway.
    ++	 */
     +	for (int i = 0; i < iovcnt; i++) {
    -+		const char *bytes = iov[i].iov_base;
    -+		size_t iovec_written = 0;
    -+
    -+		while (iovec_written < iov[i].iov_len) {
    -+			ssize_t bytes_written = xwrite(fd, bytes + iovec_written,
    -+						       iov[i].iov_len - iovec_written);
    -+			if (bytes_written < 0) {
    -+				if (total_written)
    -+					goto out;
    -+				return bytes_written;
    -+			}
    -+			if (!bytes_written)
    -+				goto out;
    -+			iovec_written += bytes_written;
    -+			total_written += bytes_written;
    -+		}
    ++		if (!iov[i].iov_len)
    ++			continue;
    ++		return xwrite(fd, iov[i].iov_base, iov[i].iov_len);
     +	}
     +
    -+out:
    -+	return (ssize_t) total_written;
    ++	/* When all iovec members were zero we ought to return 0 according to POSIX. */
    ++	return 0;
     +}
     
      ## config.mak.uname ##
2:  f6013a18ba = 2:  41814b6668 wrapper: introduce writev(3p) wrappers
3:  3dc0eff00b = 3:  af2e351491 wrapper: properly handle MAX_IO_SIZE in writev(3p)
4:  95c872432f = 4:  d3ba9d73d2 sideband: use writev(3p) to send pktlines
5:  4c1efb5284 = 5:  148a2c8928 fast-import: use writev(3p) to send cat-blob responses

---
base-commit: 55526a18268bbc1ddaf8a6b7850c33d984eac9e9
change-id: 20260714-pks-reintroduce-writev-2d8f7e52eee9

[PATCH v2 1/5] compat/posix: introduce writev(3p) wrapper

[Patrick Steinhardt]

In a subsequent commit we're going to add the first caller to
writev(3p). Introduce a compatibility wrapper for this syscall that we
can use on systems that don't have this syscall.

The syscall exists on modern Unixes like Linux and macOS, and seemingly
even for NonStop according to [1]. It doesn't seem to exist on Windows
though.

[1]: http://nonstoptools.com/manuals/OSS-SystemCalls.pdf
[2]: https://www.gnu.org/software/gnulib/manual/html_node/writev.html

Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Makefile                            |  4 ++++
 compat/posix.h                      | 14 +++++++++++++
 compat/writev.c                     | 41 +++++++++++++++++++++++++++++++++++++
 config.mak.uname                    |  2 ++
 contrib/buildsystems/CMakeLists.txt |  6 +++++-
 meson.build                         |  1 +
 6 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 1f3f099f5c..eda5ecc5b4 100644
--- a/Makefile
+++ b/Makefile
@@ -2033,6 +2033,10 @@ ifdef NO_PREAD
 	COMPAT_CFLAGS += -DNO_PREAD
 	COMPAT_OBJS += compat/pread.o
 endif
+ifdef NO_WRITEV
+	COMPAT_CFLAGS += -DNO_WRITEV
+	COMPAT_OBJS += compat/writev.o
+endif
 ifdef NO_FAST_WORKING_DIRECTORY
 	BASIC_CFLAGS += -DNO_FAST_WORKING_DIRECTORY
 endif
diff --git a/compat/posix.h b/compat/posix.h
index e2e794cad7..71cc731620 100644
--- a/compat/posix.h
+++ b/compat/posix.h
@@ -148,6 +148,9 @@
 #include <sys/socket.h>
 #include <sys/ioctl.h>
 #include <sys/statvfs.h>
+#ifndef NO_WRITEV
+#include <sys/uio.h>
+#endif
 #include <termios.h>
 #ifndef NO_SYS_SELECT_H
 #include <sys/select.h>
@@ -334,6 +337,17 @@ int git_lstat(const char *, struct stat *);
 ssize_t git_pread(int fd, void *buf, size_t count, off_t offset);
 #endif
 
+#ifdef NO_WRITEV
+#define writev git_writev
+#define iovec git_iovec
+struct git_iovec {
+	void *iov_base;
+	size_t iov_len;
+};
+
+ssize_t git_writev(int fd, const struct iovec *iov, int iovcnt);
+#endif
+
 #ifdef NO_SETENV
 #define setenv gitsetenv
 int gitsetenv(const char *, const char *, int);
diff --git a/compat/writev.c b/compat/writev.c
new file mode 100644
index 0000000000..540f66de61
--- /dev/null
+++ b/compat/writev.c
@@ -0,0 +1,41 @@
+#include "../git-compat-util.h"
+#include "../wrapper.h"
+
+ssize_t git_writev(int fd, const struct iovec *iov, int iovcnt)
+{
+	size_t sum = 0;
+
+	if (iovcnt <= 0) {
+		errno = EINVAL;
+		return -1;
+	}
+
+	/*
+	 * According to writev(3p), the syscall shall error with EINVAL in case
+	 * the sum of `iov_len` overflows `ssize_t`.
+	 */
+	for (int i = 0; i < iovcnt; i++) {
+		if (iov[i].iov_len > maximum_signed_value_of_type(ssize_t) ||
+		    unsigned_add_overflows(iov[i].iov_len, sum) ||
+		    iov[i].iov_len + sum > maximum_signed_value_of_type(ssize_t)) {
+			errno = EINVAL;
+			return -1;
+		}
+
+		sum += iov[i].iov_len;
+	}
+
+	/*
+	 * We only ever write the first non-empty vector so that we can
+	 * guarantee the call to be non-interleaving as guaranteed by POSIX.
+	 * This works just fine as callers have to loop around writev anyway.
+	 */
+	for (int i = 0; i < iovcnt; i++) {
+		if (!iov[i].iov_len)
+			continue;
+		return xwrite(fd, iov[i].iov_base, iov[i].iov_len);
+	}
+
+	/* When all iovec members were zero we ought to return 0 according to POSIX. */
+	return 0;
+}
diff --git a/config.mak.uname b/config.mak.uname
index 9ebd240378..95ef6e64dc 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -483,6 +483,7 @@ ifeq ($(uname_S),Windows)
 	SANE_TOOL_PATH ?= $(msvc_bin_dir_msys)
 	HAVE_ALLOCA_H = YesPlease
 	NO_PREAD = YesPlease
+	NO_WRITEV = YesPlease
 	NEEDS_CRYPTO_WITH_SSL = YesPlease
 	NO_LIBGEN_H = YesPlease
 	NO_POLL = YesPlease
@@ -697,6 +698,7 @@ ifeq ($(uname_S),MINGW)
 	pathsep = ;
 	HAVE_ALLOCA_H = YesPlease
 	NO_PREAD = YesPlease
+	NO_WRITEV = YesPlease
 	NEEDS_CRYPTO_WITH_SSL = YesPlease
 	NO_LIBGEN_H = YesPlease
 	NO_POLL = YesPlease
diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index a57c4b464f..8f56203f34 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -378,7 +378,7 @@ endif()
 #function checks
 set(function_checks
 	strcasestr memmem strlcpy strtoimax strtoumax strtoull
-	setenv mkdtemp poll pread memmem)
+	setenv mkdtemp poll pread memmem writev)
 
 #unsetenv,hstrerror are incompatible with windows build
 if(NOT WIN32)
@@ -423,6 +423,10 @@ if(NOT HAVE_MEMMEM)
 	list(APPEND compat_SOURCES compat/memmem.c)
 endif()
 
+if(NOT HAVE_WRITEV)
+	list(APPEND compat_SOURCES compat/writev.c)
+endif()
+
 if(NOT WIN32)
 	if(NOT HAVE_UNSETENV)
 		list(APPEND compat_SOURCES compat/unsetenv.c)
diff --git a/meson.build b/meson.build
index ca235801cf..613828ff25 100644
--- a/meson.build
+++ b/meson.build
@@ -1446,6 +1446,7 @@ checkfuncs = {
   'initgroups' : [],
   'strtoumax' : ['strtoumax.c', 'strtoimax.c'],
   'pread' : ['pread.c'],
+  'writev' : ['writev.c'],
 }
 
 if host_machine.system() == 'windows'

-- 
2.55.0.679.g6767b8d81c.dirty

[PATCH v2 2/5] wrapper: introduce writev(3p) wrappers

[Patrick Steinhardt]

In the preceding commit we have added a compatibility wrapper for the
writev(3p) syscall. Introduce some generic wrappers for this function
that we nowadays take for granted in the Git codebase.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 wrapper.c      | 41 +++++++++++++++++++++++++++++++++++++++++
 wrapper.h      |  9 +++++++++
 write-or-die.c |  8 ++++++++
 write-or-die.h |  1 +
 4 files changed, 59 insertions(+)

diff --git a/wrapper.c b/wrapper.c
index 16f5a63fbb..be8fa575e6 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -323,6 +323,47 @@ ssize_t write_in_full(int fd, const void *buf, size_t count)
 	return total;
 }
 
+ssize_t writev_in_full(int fd, struct iovec *iov, int iovcnt)
+{
+	ssize_t total_written = 0;
+
+	while (iovcnt) {
+		ssize_t bytes_written = writev(fd, iov, iovcnt);
+		if (bytes_written < 0) {
+			if (errno == EINTR || errno == EAGAIN)
+				continue;
+			return -1;
+		}
+		if (!bytes_written) {
+			errno = ENOSPC;
+			return -1;
+		}
+
+		total_written += bytes_written;
+
+		/*
+		 * We first need to discard any iovec entities that have been
+		 * fully written.
+		 */
+		while (iovcnt && (size_t)bytes_written >= iov->iov_len) {
+			bytes_written -= iov->iov_len;
+			iov++;
+			iovcnt--;
+		}
+
+		/*
+		 * Finally, we need to adjust the last iovec in case we have
+		 * performed a partial write.
+		 */
+		if (iovcnt && bytes_written) {
+			iov->iov_base = (char *) iov->iov_base + bytes_written;
+			iov->iov_len -= bytes_written;
+		}
+	}
+
+	return total_written;
+}
+
 ssize_t pread_in_full(int fd, void *buf, size_t count, off_t offset)
 {
 	char *p = buf;
diff --git a/wrapper.h b/wrapper.h
index 15ac3bab6e..27519b32d1 100644
--- a/wrapper.h
+++ b/wrapper.h
@@ -47,6 +47,15 @@ ssize_t read_in_full(int fd, void *buf, size_t count);
 ssize_t write_in_full(int fd, const void *buf, size_t count);
 ssize_t pread_in_full(int fd, void *buf, size_t count, off_t offset);
 
+/*
+ * Try to write all iovecs. Returns -1 in case an error occurred with a proper
+ * errno set, the number of bytes written otherwise.
+ *
+ * Note that the iovec will be modified as a result of this call to adjust for
+ * partial writes!
+ */
+ssize_t writev_in_full(int fd, struct iovec *iov, int iovcnt);
+
 static inline ssize_t write_str_in_full(int fd, const char *str)
 {
 	return write_in_full(fd, str, strlen(str));
diff --git a/write-or-die.c b/write-or-die.c
index 01a9a51fa2..5f522fb728 100644
--- a/write-or-die.c
+++ b/write-or-die.c
@@ -96,6 +96,14 @@ void write_or_die(int fd, const void *buf, size_t count)
 	}
 }
 
+void writev_or_die(int fd, struct iovec *iov, int iovlen)
+{
+	if (writev_in_full(fd, iov, iovlen) < 0) {
+		check_pipe(errno);
+		die_errno("writev error");
+	}
+}
+
 void fwrite_or_die(FILE *f, const void *buf, size_t count)
 {
 	if (fwrite(buf, 1, count, f) != count)
diff --git a/write-or-die.h b/write-or-die.h
index ff0408bd84..a045bdfaef 100644
--- a/write-or-die.h
+++ b/write-or-die.h
@@ -7,6 +7,7 @@ void fprintf_or_die(FILE *, const char *fmt, ...);
 void fwrite_or_die(FILE *f, const void *buf, size_t count);
 void fflush_or_die(FILE *f);
 void write_or_die(int fd, const void *buf, size_t count);
+void writev_or_die(int fd, struct iovec *iov, int iovlen);
 
 /*
  * These values are used to help identify parts of a repository to fsync.

-- 
2.55.0.679.g6767b8d81c.dirty

[PATCH v2 3/5] wrapper: properly handle MAX_IO_SIZE in writev(3p)

[Patrick Steinhardt]

Some systems like NonStop set a comparatively small `MAX_IO_SIZE`, which
limits the maximum number of bytes we're allowed to write in a single
call. We already handle this limit properly in `xwrite()`, but we have
recently introduced wrappers for writev(3p) where we don't. This will
cause the syscall to return EINVAL in case somebody passes an iovec
entry to writev(3p) that is larger than `MAX_IO_SIZE`.

Introduce a new function `xwritev()` that is similar to `xwrite()` in
that it handles such platform-specific nuances:

  - We only pass the leading iovec entries to writev(3p) that fit into
    `MAX_IO_SIZE`, pretending that the underlying syscall performed a
    short write. This mirrors how `xwrite()` chomps overly large
    requests before handing them to write(3p). As a consequence, callers
    will never see writev(3p)'s EINVAL error for requests whose summed
    length would overflow an ssize_t, but observe a short write instead.

  - If already the first iovec entry exceeds the limit we instead punt
    to `xwrite()`, which knows to handle this case for us.

  - We restart the underlying syscall on EINTR and EAGAIN, just like
    `xwrite()` does for write(3p).

Adapt `writev_in_full()` to use this new wrapper. With the retry logic
now living in `xwritev()`, the calling loop becomes the exact mirror
image of `write_in_full()`, which also retains the responsibility of
translating a zero-length write into ENOSPC.

Reported-by: Randall Becker <randall.becker@nexbridge.ca>
Helped-by: Jeff King <peff@peff.net>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 wrapper.c | 47 ++++++++++++++++++++++++++++++++++++++++++-----
 wrapper.h |  1 +
 2 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/wrapper.c b/wrapper.c
index be8fa575e6..561f9ee9c9 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -323,17 +323,54 @@ ssize_t write_in_full(int fd, const void *buf, size_t count)
 	return total;
 }
 
+ssize_t xwritev(int fd, struct iovec *iov, int iovcnt)
+{
+	size_t allowed = MAX_IO_SIZE;
+	int i;
+
+	/*
+	 * Some platforms define a comparatively small `MAX_IO_SIZE` that
+	 * limits how many bytes can be written with a single call to
+	 * write(3p) or writev(3p); exceeding that limit causes the syscall to
+	 * fail with EINVAL. Just like xwrite() chomps overly large requests
+	 * for write(3p), pretend that the underlying writev(3p) performed a
+	 * short write by only passing along the leading iovec entries that
+	 * fit into that limit.
+	 */
+	for (i = 0; i < iovcnt; i++) {
+		if (iov[i].iov_len > allowed) {
+			/*
+			 * If the first buffer is larger than MAX_IO_SIZE,
+			 * let xwrite() deal with it.
+			 */
+			if (!i)
+				return xwrite(fd, iov->iov_base, iov->iov_len);
+			break;
+		}
+		allowed -= iov[i].iov_len;
+	}
+
+	while (1) {
+		ssize_t bytes_written = writev(fd, iov, i);
+		if (bytes_written < 0) {
+			if (errno == EINTR)
+				continue;
+			if (handle_nonblock(fd, POLLOUT, errno))
+				continue;
+		}
+
+		return bytes_written;
+	}
+}
+
 ssize_t writev_in_full(int fd, struct iovec *iov, int iovcnt)
 {
 	ssize_t total_written = 0;
 
 	while (iovcnt) {
-		ssize_t bytes_written = writev(fd, iov, iovcnt);
-		if (bytes_written < 0) {
-			if (errno == EINTR || errno == EAGAIN)
-				continue;
+		ssize_t bytes_written = xwritev(fd, iov, iovcnt);
+		if (bytes_written < 0)
 			return -1;
-		}
 		if (!bytes_written) {
 			errno = ENOSPC;
 			return -1;
diff --git a/wrapper.h b/wrapper.h
index 27519b32d1..a6287d7f4d 100644
--- a/wrapper.h
+++ b/wrapper.h
@@ -16,6 +16,7 @@ void *xmmap_gently(void *start, size_t length, int prot, int flags, int fd, off_
 int xopen(const char *path, int flags, ...);
 ssize_t xread(int fd, void *buf, size_t len);
 ssize_t xwrite(int fd, const void *buf, size_t len);
+ssize_t xwritev(int fd, struct iovec *iov, int iovcnt);
 ssize_t xpread(int fd, void *buf, size_t len, off_t offset);
 int xdup(int fd);
 FILE *xfopen(const char *path, const char *mode);

-- 
2.55.0.679.g6767b8d81c.dirty

[PATCH v2 4/5] sideband: use writev(3p) to send pktlines

[Patrick Steinhardt]

Every pktline that we send out via `send_sideband()` currently requires
two syscalls: one to write the pktline's length, and one to send its
data. This typically isn't all that much of a problem, but under extreme
load the syscalls may cause contention in the kernel.

Refactor the code to instead use the newly introduced writev(3p) infra
so that we can send out the data with a single syscall. This reduces the
number of syscalls from around 133,000 calls to write(3p) to around
67,000 calls to writev(3p).

This change leads to a performance improvement for git-upload-pack(1),
but we have to cheat a bit to really make it measurable. Usually, the
time is strongly dominated by generating the packfile itself. But if we
precompute the pack and serve it via the pack-objects hook then we can
essentially eliminate that overhead. The following setup is executed in
the Git repository:

  $ cat >request <<-EOF
  0048want 5ce91c059e41090e7d2cffad39c04af8acf98dc1 side-band no-progress
  00000009done
  EOF
  $ echo 5ce91c059e41090e7d2cffad39c04af8acf98dc1 | git pack-objects --revs --stdout >pack
  $ cat >hook <<-EOF
  #!/bin/sh
  cat >/dev/null
  cat "$(pwd)"/pack
  EOF
  $ chmod u+x hook
  $ git -c uploadpack.packObjectsHook="$(pwd)"/hook upload-pack . <request

Benchmarking the last command leads to the following results:

  Benchmark 1: HEAD~
    Time (mean ± σ):     192.9 ms ±   0.6 ms    [User: 106.5 ms, System: 95.3 ms]
    Range (min … max):   191.7 ms … 194.1 ms    50 runs

  Benchmark 2: HEAD
    Time (mean ± σ):     141.1 ms ±   0.7 ms    [User: 63.2 ms, System: 86.6 ms]
    Range (min … max):   139.8 ms … 142.7 ms    50 runs

  Summary
    HEAD ran
      1.37 ± 0.01 times faster than HEAD~

This might not be impressive in absolute numbers when you also take into
account the time it takes to generate the packfile itself. But GitLab
(and supposedly other forges) have caching mechanisms in place that work
exactly like the above setup, where repeated incoming requests can be
served from the same cached packfile. And in those cases, the impact is
sizeable.

More importantly though, as hinted at above, GitLab has observed in the
past that with enough cache hits we eventually start to saturate a
semaphore in the Linux kernel itself in the pipe write path. This
bottleneck is being moved a bit by having to do less syscalls.

Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 sideband.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/sideband.c b/sideband.c
index 1523a53e1d..94e5b56172 100644
--- a/sideband.c
+++ b/sideband.c
@@ -441,6 +441,7 @@ void send_sideband(int fd, int band, const char *data, ssize_t sz, int packet_ma
 	const char *p = data;
 
 	while (sz) {
+		struct iovec iov[2];
 		unsigned n;
 		char hdr[5];
 
@@ -450,12 +451,19 @@ void send_sideband(int fd, int band, const char *data, ssize_t sz, int packet_ma
 		if (0 <= band) {
 			xsnprintf(hdr, sizeof(hdr), "%04x", n + 5);
 			hdr[4] = band;
-			write_or_die(fd, hdr, 5);
+			iov[0].iov_base = hdr;
+			iov[0].iov_len = 5;
 		} else {
 			xsnprintf(hdr, sizeof(hdr), "%04x", n + 4);
-			write_or_die(fd, hdr, 4);
+			iov[0].iov_base = hdr;
+			iov[0].iov_len = 4;
 		}
-		write_or_die(fd, p, n);
+
+		iov[1].iov_base = (void *) p;
+		iov[1].iov_len = n;
+
+		writev_or_die(fd, iov, ARRAY_SIZE(iov));
+
 		p += n;
 		sz -= n;
 	}

-- 
2.55.0.679.g6767b8d81c.dirty

[PATCH v2 5/5] fast-import: use writev(3p) to send cat-blob responses

[Patrick Steinhardt]

When answering a `cat-blob` command, `cat_blob()` issues three separate
calls to write(3p) on the cat-blob fd: one for the header line, one for
the full blob payload, and one for the trailing newline. Frontends like
git-filter-repo issue these commands in bulk, once per rewritten blob,
so the syscall overhead adds up.

Use `writev_in_full()` to send all three parts with a single syscall.

This can be benchmarked with the following setup:

    $ git cat-file --unordered --filter=object:type=blob
        --batch-check='cat-blob %(objectname)' --batch-all-objects >request
    $ git fast-import --cat-blob-fd=3 <request

Executing this with 100,000 objects in linux.git:

  Benchmark 1: HEAD~
    Time (mean ± σ):      1.320 s ±  0.003 s    [User: 1.154 s, System: 0.161 s]
    Range (min … max):    1.314 s …  1.324 s    10 runs

  Benchmark 2: HEAD
    Time (mean ± σ):      1.270 s ±  0.022 s    [User: 1.133 s, System: 0.132 s]
    Range (min … max):    1.209 s …  1.282 s    10 runs

  Summary
    HEAD ran
      1.04 ± 0.02 times faster than HEAD~

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/fast-import.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index aa656c5195..48fda01c94 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -3332,6 +3332,7 @@ static void cat_blob_write(const char *buf, unsigned long size)
 static void cat_blob(struct object_entry *oe, struct object_id *oid)
 {
 	struct strbuf line = STRBUF_INIT;
+	struct iovec iov[3];
 	unsigned long size;
 	enum object_type type = 0;
 	char *buf;
@@ -3365,10 +3366,21 @@ static void cat_blob(struct object_entry *oe, struct object_id *oid)
 	strbuf_reset(&line);
 	strbuf_addf(&line, "%s %s %"PRIuMAX"\n", oid_to_hex(oid),
 		    type_name(type), (uintmax_t)size);
-	cat_blob_write(line.buf, line.len);
+
+	/*
+	 * Write the header, the payload and the trailing newline with a
+	 * single writev(3p) call instead of three separate write(3p) calls.
+	 */
+	iov[0].iov_base = line.buf;
+	iov[0].iov_len = line.len;
+	iov[1].iov_base = buf;
+	iov[1].iov_len = size;
+	iov[2].iov_base = (void *) "\n";
+	iov[2].iov_len = 1;
+
+	if (writev_in_full(cat_blob_fd, iov, ARRAY_SIZE(iov)) < 0)
+		die_errno(_("write to frontend failed"));
 	strbuf_release(&line);
-	cat_blob_write(buf, size);
-	cat_blob_write("\n", 1);
 	if (oe && oe->pack_id == pack_id) {
 		last_blob.offset = oe->idx.offset;
 		strbuf_attach(&last_blob.data, buf, size, size + 1);

-- 
2.55.0.679.g6767b8d81c.dirty

Re: [PATCH 0/5] Reintroduce writev(3p)

[Patrick Steinhardt]

On Thu, Aug 06, 2026 at 01:26:21PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > On Wed, Aug 05, 2026 at 01:29:44PM -0700, Junio C Hamano wrote:
> >> Johannes Sixt <j6t@kdbg.org> writes:
> >> 
> >> > Am 05.08.26 um 20:40 schrieb Junio C Hamano:
> >> >> I think it is OK to explicitly document that any writev(2) emulation
> >> >> is allowed to be non-atomic, and it is also OK to declare that using
> >> >> writev(2) in this application to allow competing writes to the same
> >> >> destination is a bug.
> >> >
> >> > These are fine.
> >> >
> >> > But I'm not worried about current uses of writev, I'm worried about
> >> > future uses: "Look, we already use writev elsewhere. Let's use it here,
> >> > too, where we can take adavantage of the atomicity of the write." It's
> >> > too easy to miss a note about non-atomic emulations when the function
> >> > name advertises more than can be guaranteed. For this reason, I strongly
> >> > suggest to use a different name.
> >> 
> >> That is why I added the "it is also OK to declare" in the above.
> >
> > We could of course trivially restore the non-interleaving property by
> > only ever writing the first iovec. POSIX doesn't guarantee that the full
> > iovec is being written, and write(3p) is already non-interleaving. It
> > wouldn't even be less efficient compared to the current implementation,
> > as we have to loop around write(3p) anyway in our compatibility wrapper.
> 
> OK, by castrating the writev(2) emulation implementation to write
> out only the first iovec[], we are making the emulation "atomic", so
> there is no need to say "your emulation does not have to be atomic"
> and we can rely on being able to pretend that we have writev(2)
> available everywhere.  Also, it is a bug on the programmers' side to
> assume that their writev() calls will not result in a short write,
> so it does not have to be spelled out, either, which automatically
> means you'd better be calling writev_in_full() and not writev()
> itself.
> 
> I can buy that.  Clever.  It means we'd need an update for [PATCH
> 1/5] 1ed0bc4e3b (compat/posix: introduce writev(3p) wrapper,
> 2026-07-16), right?  The update would be a simplification that loses
> a lot of code (and overflow check), which is even nicer ;-).

Yeah, exactly. The overflow check I think we should keep though to be
closer to the POSIX requirements.

Patrick