From: Jeff King <peff@peff.net>
To: Patrick Steinhardt <ps@pks.im>
Cc: "brian m. carlson" <sandals@crustytoothpaste.net>,
Junio C Hamano <gitster@pobox.com>,
rsbecker@nexbridge.com, git@vger.kernel.org
Subject: Re: Git 2.54.0-rc1, subtests of t5310, t5326, t5327
Date: Thu, 9 Apr 2026 16:51:45 -0400 [thread overview]
Message-ID: <20260409205145.GC3076846@coredump.intra.peff.net> (raw)
In-Reply-To: <addgkjiB80pgKw69@pks.im>
On Thu, Apr 09, 2026 at 10:17:22AM +0200, Patrick Steinhardt wrote:
> > that people should be able to send. I'd personally expect to be able to
> > send values much larger, at least 512 KiB, and I have code that expects
> > even larger (16 MiB).
> >
> > So I'd simply say that for systems that have a constraint on the size
> > that is "too small", they should just use NO_WRITEV.
>
> I would be happy with this as an intermediate step, as Randall has
> confirmed it would fix the issue. It is the least intrusive step and has
> the lowest risk.
The only downside is that if there are other systems with a small
SSIZE_MAX, they'll need similar treatment.
I took a look at auto-detecting this at the preprocessor stage. The
conditional itself is not too bad, but we have to compile writev.o
unconditionally, as well as re-order some definitions:
diff --git a/Makefile b/Makefile
index 5d22394c2e..f9b4dd8ae3 100644
--- a/Makefile
+++ b/Makefile
@@ -1125,6 +1125,7 @@ LIB_OBJS += compat/nonblock.o
LIB_OBJS += compat/obstack.o
LIB_OBJS += compat/open.o
LIB_OBJS += compat/terminal.o
+LIB_OBJS += compat/writev.o
LIB_OBJS += compiler-tricks/not-constant.o
LIB_OBJS += config.o
LIB_OBJS += connect.o
@@ -2031,7 +2032,6 @@ ifdef NO_PREAD
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
diff --git a/compat/posix.h b/compat/posix.h
index 94699a03fa..57c11938c7 100644
--- a/compat/posix.h
+++ b/compat/posix.h
@@ -326,6 +326,39 @@ int git_lstat(const char *, struct stat *);
ssize_t git_pread(int fd, void *buf, size_t count, off_t offset);
#endif
+/*
+ * Limit size of IO chunks, because huge chunks only cause pain. OS X
+ * 64-bit is buggy, returning EINVAL if len >= INT_MAX; and even in
+ * the absence of bugs, large chunks can result in bad latencies when
+ * you decide to kill the process.
+ *
+ * We pick 8 MiB as our default, but if the platform defines SSIZE_MAX
+ * that is smaller than that, clip it to SSIZE_MAX, as a call to
+ * read(2) or write(2) larger than that is allowed to fail. As the last
+ * resort, we allow a port to pass via CFLAGS e.g. "-DMAX_IO_SIZE=value"
+ * to override this, if the definition of SSIZE_MAX given by the platform
+ * is broken.
+ */
+#ifndef MAX_IO_SIZE
+# define MAX_IO_SIZE_DEFAULT (8*1024*1024)
+# if defined(SSIZE_MAX) && (SSIZE_MAX < MAX_IO_SIZE_DEFAULT)
+# define MAX_IO_SIZE SSIZE_MAX
+# else
+# define MAX_IO_SIZE MAX_IO_SIZE_DEFAULT
+# endif
+#endif
+
+/*
+ * Temporary hack: our use of writev() does not respect MAX_IO_SIZE, but we
+ * never feed more than a single pkt-line packet to it. Disable writev()
+ * and use our fallback wrapper if the system can't support that size.
+ *
+ * This should be removed once writev() supports MAX_IO_SIZE.
+ */
+#if !defined(NO_WRITEV) && (MAX_IO_SIZE < 65536)
+#define NO_WRITEV
+#endif
+
#ifdef NO_WRITEV
#define writev git_writev
#define iovec git_iovec
diff --git a/git-compat-util.h b/git-compat-util.h
index 4b4ea2498f..60108459f3 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -677,28 +677,6 @@ static inline uint64_t u64_add(uint64_t a, uint64_t b)
return a + b;
}
-/*
- * Limit size of IO chunks, because huge chunks only cause pain. OS X
- * 64-bit is buggy, returning EINVAL if len >= INT_MAX; and even in
- * the absence of bugs, large chunks can result in bad latencies when
- * you decide to kill the process.
- *
- * We pick 8 MiB as our default, but if the platform defines SSIZE_MAX
- * that is smaller than that, clip it to SSIZE_MAX, as a call to
- * read(2) or write(2) larger than that is allowed to fail. As the last
- * resort, we allow a port to pass via CFLAGS e.g. "-DMAX_IO_SIZE=value"
- * to override this, if the definition of SSIZE_MAX given by the platform
- * is broken.
- */
-#ifndef MAX_IO_SIZE
-# define MAX_IO_SIZE_DEFAULT (8*1024*1024)
-# if defined(SSIZE_MAX) && (SSIZE_MAX < MAX_IO_SIZE_DEFAULT)
-# define MAX_IO_SIZE SSIZE_MAX
-# else
-# define MAX_IO_SIZE MAX_IO_SIZE_DEFAULT
-# endif
-#endif
-
#ifdef HAVE_ALLOCA_H
# include <alloca.h>
# define xalloca(size) (alloca(size))
It's not _too_ complicated, but it's a little more than I'd like for an
-rc2. Yet another option is to swap out NO_WRITEV for USE_WRITEV,
effectively reverting the feature temporarily until it supports
MAX_IO_SIZE. But that's also slightly non-trivial.
So I dunno. Do we want just:
diff --git a/config.mak.uname b/config.mak.uname
index ccb3f71881..b672b1d077 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -654,6 +654,7 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
CSPRNG_METHOD = openssl
SANE_TOOL_PATH = /usr/coreutils/bin:/usr/local/bin
SHELL_PATH = /usr/coreutils/bin/bash
+ NO_WRITEV = NotUntilItSupportsMaxIOSize
endif
ifeq ($(uname_S),OS/390)
NO_SYS_POLL_H = YesPlease
and hope it's the only one?
-Peff
next prev parent reply other threads:[~2026-04-09 20:51 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-07 23:29 Git 2.54.0-rc1, subtests of t5310, t5326, t5327 rsbecker
2026-04-08 4:17 ` Jeff King
2026-04-08 14:54 ` rsbecker
2026-04-08 16:25 ` rsbecker
2026-04-08 17:39 ` Jeff King
2026-04-08 18:12 ` Junio C Hamano
2026-04-08 20:08 ` rsbecker
2026-04-08 20:21 ` Junio C Hamano
2026-04-08 21:27 ` rsbecker
2026-04-08 21:43 ` Junio C Hamano
2026-04-08 22:04 ` rsbecker
2026-04-08 22:24 ` Junio C Hamano
2026-04-08 22:35 ` Junio C Hamano
2026-04-08 23:15 ` rsbecker
2026-04-08 22:32 ` Jeff King
2026-04-09 0:20 ` brian m. carlson
2026-04-09 8:17 ` Patrick Steinhardt
2026-04-09 9:48 ` Phillip Wood
2026-04-09 11:29 ` Patrick Steinhardt
2026-04-09 13:46 ` rsbecker
2026-04-09 20:33 ` Jeff King
2026-04-09 22:40 ` rsbecker
2026-04-09 22:58 ` Jeff King
2026-04-10 4:34 ` Patrick Steinhardt
2026-04-09 20:51 ` Jeff King [this message]
2026-04-10 7:35 ` Johannes Sixt
2026-04-08 18:36 ` rsbecker
2026-04-08 22:14 ` Jeff King
2026-04-08 17:37 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260409205145.GC3076846@coredump.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=ps@pks.im \
--cc=rsbecker@nexbridge.com \
--cc=sandals@crustytoothpaste.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox