From: Jeff King Date: Thu, 09 Apr 2026 20:51:45 GMT Subject: Re: Git 2.54.0-rc1, subtests of t5310, t5326, t5327 Message-ID: <20260409205145.GC3076846@coredump.intra.peff.net> In-Reply-To: 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 # 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