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

On June 4, 2021 1:12 AM, Junio C Hamano wrote:

To: Jeff King <redacted>
Cc: Taylor Blau <redacted>; Randall S. Becker <redacted>; git@vger.kernel.org
Subject: Re: [ANNOUNCE] Git v2.32.0-rc3 - t5300 Still Broken on NonStop ia64/x86

Jeff King [off-list ref] writes:

quoted

One brief aside: I'm still not entirely convinced that NonStop isn't
violating POSIX. Yes, as Eric noted, fsync() is allowed to return EINTR.
But should it do so when the signal it got was set up with SA_RESTART?

The sigaction(3posix) page says:

     SA_RESTART   This flag affects the behavior of interruptible functions;
		  that is, those specified to fail with errno set to
		  [EINTR]. If set, and a function specified as
		  interruptible is interrupted by this signal, the
		  function shall restart and shall not fail with [EINTR]
		  unless otherwise specified. [...]

and I could not find anywhere that it is "otherwise specified" for
fsync(). Of course, whatever POSIX says, if NonStop needs this
workaround, we should provide it. But this may explain why we never
saw it on other systems.

Yeah, I think all of the above makes sense.

quoted

It also means it's less important for this workaround to kick in
everywhere. But given how low-cost it is, I'm just as happy to avoid
having a separate knob to enable it.

Yes, any caller who cares about the result of fsync() is willing to wait, and on a well benaved system, we would never see EINTR so

the

loop won't iterate.  Having to carry just a handful of extra bytes in ICache in a codepath that is not performance critical is

probably an

acceptable cost for simpler code.

Just something to note: I was running git gc --aggressive on OpenSSL today and got the same error as in this case. I'm assuming that
the fix will apply to that too. In this situation, the gc ran for over 2 hours until failing. This may be a more widely available
test condition for fsync() EINTR, possibly.

-Randall