git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Taylor Blau <me@ttaylorr.com>,
	"Randall S. Becker" <rsbecker@nexbridge.com>,
	git@vger.kernel.org
Subject: Re: [ANNOUNCE] Git v2.32.0-rc3 - t5300 Still Broken on NonStop ia64/x86
Date: Fri, 04 Jun 2021 10:36:11 +0900	[thread overview]
Message-ID: <xmqqk0na2yyc.fsf@gitster.g> (raw)
In-Reply-To: <YLfm8cqY6EjQuhcO@coredump.intra.peff.net> (Jeff King's message of "Wed, 2 Jun 2021 16:15:45 -0400")

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

  parent reply	other threads:[~2021-06-04  1:36 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-02 17:52 [ANNOUNCE] Git v2.32.0-rc3 - t5300 Still Broken on NonStop ia64/x86 Randall S. Becker
2021-06-02 19:32 ` Taylor Blau
2021-06-02 19:49   ` Jeff King
2021-06-02 20:11     ` Taylor Blau
2021-06-02 20:15       ` Jeff King
2021-06-02 20:36         ` Randall S. Becker
2021-06-04  1:36         ` Junio C Hamano [this message]
2021-06-04  2:17           ` Taylor Blau
2021-06-04  3:55           ` Jeff King
2021-06-04  5:12             ` Junio C Hamano
2021-06-06 19:06               ` Randall S. Becker
2021-06-08  6:40                 ` Jeff King
2021-06-05  7:04           ` René Scharfe
2021-06-05 13:15             ` Junio C Hamano
2021-06-02 20:11     ` Eric Wong
2021-06-02 20:14       ` Jeff King
2021-06-02 20:18         ` Taylor Blau
2021-06-02 20:34         ` Randall S. Becker
2021-06-03 19:31           ` Jeff King
2021-06-03 20:07             ` Randall S. Becker
2021-06-03 20:21         ` Bryan Turner
2021-06-03 20:32           ` Randall S. Becker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqk0na2yyc.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=me@ttaylorr.com \
    --cc=peff@peff.net \
    --cc=rsbecker@nexbridge.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).