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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.