Re: [ANNOUNCE] Git v2.32.0-rc3 - t5300 Still Broken on NonStop ia64/x86

On Wed, Jun 2, 2021 at 1:14 PM Jeff King [off-list ref] wrote:

On Wed, Jun 02, 2021 at 08:11:50PM +0000, Eric Wong wrote:

quoted

Jeff King [off-list ref] wrote:

quoted

And so when he gets this error:

  fatal: fsync error on '.git/objects/pack/tmp_pack_NkPgqN': Interrupted system call

presumably we were in fsync() when the signal arrived, and unlike most
other platforms, the call needs to be restarted manually (even though we
set up the signal with SA_RESTART). I'm not sure if this violates POSIX
or not (I couldn't find a definitive answer to the set of interruptible
functions in the standard). But either way, the workaround is probably
something like:

"man 3posix fsync" says EINTR is allowed ("manpages-posix-dev"
package in Debian non-free).

Ah, thanks. Linux's fsync(3) doesn't mention it, and nor does it appear
in the discussion of interruptible calls in signals(7). So I was looking
for a POSIX equivalent of that signals manpage but couldn't find one. :)

quoted

quoted

  #ifdef FSYNC_NEEDS_RESTART

The wrapper should apply to all platforms.  NFS (and presumably
other network FSes) can be mounted with interrupts enabled.

I don't mind that, as the wrapper is pretty low-cost (and one less
Makefile knob is nice). If it's widespread, though, I find it curious
that nobody has run into it before now.

I was dealing with a similar issue[1] recently, albeit not in the Git
codebase but rather with Java. My issue was with epoll_wait, rather
than fsync, which is documented on signal(7) as not restartable even
with SA_RESTART. That led me to this[2] little bit of code inside the
JVM:
#define RESTARTABLE(_cmd, _result) do { \
  do { \
    _result = _cmd; \
  } while((_result == -1) && (errno == EINTR)); \
} while(0)

which they use like this[3]:
RESTARTABLE(epoll_wait(epfd, events, numfds, -1), res);

Not sure what the Git maintainers' view on macros is, but if there
wasn't going to be a Makefile knob perhaps something similar might
make sense as a reusable construct. Of course, it's unclear how often
Git might _need_ such a thing; given this doesn't seem to come up
much, perhaps that's a sign such a macro would end up a waste of
effort. Anyway, just thought I'd share because I was looking at
something similar.

[1] https://github.com/brettwooldridge/NuProcess/issues/124
[2] https://github.com/JetBrains/jdk8u_jdk/blob/94318f9185757cc33d2b8d527d36be26ac6b7582/src/solaris/native/sun/nio/ch/nio_util.h#L33-L37
[3] https://github.com/JetBrains/jdk8u_jdk/blob/94318f9185757cc33d2b8d527d36be26ac6b7582/src/solaris/native/sun/nio/ch/EPoll.c#L92

-Peff