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

From: Randall S. Becker <rsbecker@nexbridge.com>
Date: Jun 2, 2021, 20:34 UTC
Message-ID: <003c01d757ee$c0664600$4132d200$@nexbridge.com>
In-Reply-To: <YLfmo8kl0URnGgp5@coredump.intra.peff.net>

On June 2, 2021 4:14 PM, Peff wrote:

Show 29 quoted lines

>Subject: Re: [ANNOUNCE] Git v2.32.0-rc3 - t5300 Still Broken on NonStop ia64/x86
>On Wed, Jun 02, 2021 at 08:11:50PM +0000, Eric Wong wrote:
>
>> Jeff King <peff@peff.net> wrote:
>> > 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. :)
>
>> >   #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 suspect this is because of the way the file system on NonStop behaves. It is a multi-processor platform, with multi-cores, so anything can happen. If the file system is delayed for any reason, like a signal coming from a different core (EINTR has high priority), then fsync() will be interrupted. EINTR is allowed on NonStop for fsync(). So it would be really great if the patch included a modification to config.mak.uname to include that. This would be a timing-only issue on most other systems, probably something that would hit NFS.

The patch for the config is:

diff --git a/config.mak.uname b/config.mak.uname
index cb443b4e02..ac3e3ca2c5 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -566,6 +566,7 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
        NO_REGEX = NeedsStartEnd
        NO_PTHREADS = UnfortunatelyYes
        FREAD_READS_DIRECTORIES = UnfortunatelyYes
+       FSYNC_NEEDS_RESTART = YesPlease

        # Not detected (nor checked for) by './configure'.