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

On Fri, Jun 04, 2021 at 10:36:11AM +0900, Junio C Hamano wrote:

Jeff King [off-list ref] writes:

quoted

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.

That makes total sense to me; I can't imagine a scenario where you
would want to call fsync() over fsync_or_die(). But if you did, then you
probably don't care about whether or not fsync() was interrupted, or can
check the return value yourself.

If it became more common, then I wouldn't mind #undef-ing fsync() and
replacing it with our own hacked up version.

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.

Your patch looks good to me (and Randall already tested my version with
the additional knob on NonStop and reported it working as expected[1]),
so this has my:

  Reviewed-by: Taylor Blau [off-list ref]

Thanks,
Taylor

[1]: https://lore.kernel.org/git/009901d758b4$12016d80$36044880$@nexbridge.com/ (local)