On June 4, 2021 1:12 AM, Junio C Hamano wrote: >To: Jeff King <peff@xxxxxxxx> >Cc: Taylor Blau <me@xxxxxxxxxxxx>; Randall S. Becker <rsbecker@xxxxxxxxxxxxx>; git@xxxxxxxxxxxxxxx >Subject: Re: [ANNOUNCE] Git v2.32.0-rc3 - t5300 Still Broken on NonStop ia64/x86 > >Jeff King <peff@xxxxxxxx> writes: > >> 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. > >> 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
