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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff King <peff@xxxxxxxx> 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@xxxxxxxxxxxx>
Helped-by: Jeff King <peff@xxxxxxxx>
Helped-by: Taylor Blau <me@xxxxxxxxxxxx>
[jc: the above two did most of the work---I just tied the loose end]
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---

 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);
 	}
 }



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux