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

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 4, 2021, 01:36 UTC
Message-ID: <xmqqk0na2yyc.fsf@gitster.g>
In-Reply-To: <YLfm8cqY6EjQuhcO@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> This looks as I'd expect. But after seeing Eric's response, we perhaps
> want to do away with the knob entirely.

Thanks. I was hoping somebody in the thread would tie the loose ends, but upon inspection of the output from

    $ git grep -e fsync\( maint seen -- \*.[ch]

it turns out that fsync_or_die() is the only place that calls fsync(), so perhaps doing it in a way that is quite different from what has been discussed may be even a better alternative.

If any new callers care about the return value of fsync(), I'd expect that they would be calling this wrapper, and the "best effort" callers that do not check the returned value by definition do not care if fsync() does not complete due to an interrupt, so I am hoping that the current "we only call it from this wrapper" is not just "the code currently happens to be this way", but it is sensible that the code will stay that way in the future.

Obviously I appreciate reviews and possibly tests, but sanity checking my observation that fsync() is called only from here is a good thing to have.

-- >8 --
Subject: fsync(): be prepared to see EINTR

Some platforms, like NonStop do not automatically restart fsync() when interrupted by a signal, even when that signal is setup with SA_RESTART.

This can lead to test breakage, e.g., where "--progress" is used, thus SIGALRM is sent often, and can interrupt an fsync() syscall.

Make sure we deal with such a case by retrying the syscall
ourselves.  Luckily, we call fsync() fron a single wrapper,
fsync_or_die(), so the fix is fairly isolated.
    
Reported-by: Randall S. Becker <randall.becker@nexbridge.ca>
Helped-by: Jeff King <peff@peff.net>
Helped-by: Taylor Blau <me@ttaylorr.com>
[jc: the above two did most of the work---I just tied the loose end]
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 write-or-die.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git i/write-or-die.c w/write-or-die.c
index eab8c8d0b9..534b2f5cba 100644
--- i/write-or-die.c
+++ w/write-or-die.c
@@ -57,7 +57,11 @@ void fprintf_or_die(FILE *f, const char *fmt, ...)
 
 void fsync_or_die(int fd, const char *msg)
 {
-	if (fsync(fd) < 0) {
+	int status;
+
+	while ((status = fsync(fd)) < 0 && errno == EINTR)
+		; /* try again */
+	if (status < 0) {
 		die_errno("fsync error on '%s'", msg);
 	}
 }