RE: Git 2.54.0-rc1, subtests of t5310, t5326, t5327

[<rsbecker@nexbridge.com>]

On April 9, 2026 4:17 AM, Patrick Steinhardt wrote
>On Thu, Apr 09, 2026 at 12:20:26AM +0000, brian m. carlson wrote:
>> On 2026-04-08 at 22:32:33, Jeff King wrote:
>> > I think writev() is buying us something when it works (it is hlving
>> > the number of writes for sideband packets). And it works when either:
>> >
>> >   1. the platform is OK with writing up to 64k in a single writev()
>> >
>> >   2. the platform has a limit that is small (like NonStop here), but
>> >      writes less than MAX_IO_SIZE work and will save a write() call
>> >
>> > If we just care about (1), then the right solution is to declare
>> > that
>> > writev() isn't fully functional for us on some platforms, and they
>> > should build with NO_WRITEV. And we should probably embed that in
>> > config.mak.uname.
>
>Yeah, agreed. I think we shouldn't make ourselves a hostage to platforms
that don't
>have reasonable support for writev(3p), as it does buy us something on the
>majority of platforms that actually support it well.
>
>That of course doesn't mean that we shouldn't support such platforms.
>
>> Looking at POSIX, there doesn't seem to be any constraints on the size
>> of individual vectors other than that they must total to less than
>> SSIZE_MAX.  iovcnt can be limited to 16, but I don't think we're
>> hitting that here.  POSIX does say that SSIZE_MAX does not need to
>> exceed 32767, which may be what's going on here, although that does
>> seem like an unreasonable value for a real system.  Linux, FreeBSD,
>> and NetBSD all set SSIZE_MAX to either INT_MAX or LONG_MAX.
>>
>> I also think that 64 KiB is more than reasonable in terms of the size
>> 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.
>
>> However, I don't have a strong opinion on this and if people want to
>> do the proposal for option 2, that's fine with me.
>
>I think in the long term this is the most sensible approach though so that
we don't
>have to special-case platforms. I've crafted the below alternative to
Peff's patch, and
>I think it's ultimately not too bad.
>
>One question to Randall though: does MAX_IO_SIZE apply to the overall size
of the
>iovec or to the individual iovec entries? I think it should be the latter,
but I cannot
>easily verify and couldn't find any docs around this. So could you please
try the
>patch at the end of this mail to verify that it works on your system?
>
>In any case, I've tested that my patch also works when defining MAX_IO_SIZE
to
>128 bytes on my system, which hopefully demonstrates that it works as
expected:
>
>diff --git a/git-compat-util.h b/git-compat-util.h index
4b4ea2498f..8e02b5f673
>100644
>--- a/git-compat-util.h
>+++ b/git-compat-util.h
>@@ -690,14 +690,8 @@ static inline uint64_t u64_add(uint64_t a, uint64_t b)
>  * 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
>+#undef MAX_IO_SIZE
>+#define MAX_IO_SIZE 128
>
> #ifdef HAVE_ALLOCA_H
> # include <alloca.h>
>
>I'm happy to go either way, but think that we should definitely aim for the
below
>patch eventually. Just let me know which way you prefer and I'm happy to
polish up
>the patch.
>
>Patrick
>
>diff --git a/wrapper.c b/wrapper.c
>index be8fa575e6..645dbc5f20 100644
>--- a/wrapper.c
>+++ b/wrapper.c
>@@ -323,21 +323,50 @@ 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) {
>+	ssize_t bytes_written;
>+	int i;
>+
>+	/*
>+	 * We need to make sure that no individual iovec entry exceeds
>+	 * `MAX_IO_SIZE`. If there's any entry that does exceed this limit
>+	 * we'll pass all entries up to it to `writev()`, and then process
the
>+	 * exceeding entry via a call to `xwrite()`.
>+	 */
>+	for (i = 0; i < iovcnt; i++)
>+		if (iov[i].iov_len > MAX_IO_SIZE)
>+			break;
>+	if (i < iovcnt) {
>+		/*
>+		 * The first entry exceeds MAX_IO_SIZE, so we pass it to
>+		 * xwrite, which knows to handle his case.
>+		 */
>+		if (!i)
>+			return xwrite(fd, iov->iov_base, iov->iov_len);
>+		iovcnt = i;
>+	}
>+
>+	bytes_written = writev(fd, iov, iovcnt);
>+	if (!bytes_written) {
>+		errno = ENOSPC;
>+		return -1;
>+	}
>+
>+	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) {
>+		ssize_t bytes_written = xwritev(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;
>
>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);

Please do not make the change in git-compat-util. This will break xwrite().
We already have MAX_IO_SIZE working and verified from years ago. Changing
that will remove our platform from being supportable.