Re: [PATCH v3 2/3] sha1_file: open window into packfiles with O_CLOEXEC

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Oct 27, 2016, 23:44 UTC
Message-ID: <CA+55aFw83E+zOd+z5h-CA-3NhrLjVr-anL6pubrSWttYx3zu8g@mail.gmail.com>
In-Reply-To: <xmqqfunh4b63.fsf@gitster.mtv.corp.google.com>

On Thu, Oct 27, 2016 at 4:36 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 5 quoted lines

>
> Would the best endgame shape for this function be to open with
> O_NOATIME (and retry without), and then add CLOEXEC with fcntl(2)
> but ignoring an error from it, I guess?  That would be the closest
> to what we historically had, I would think.

I think that's the best model.

Note that the O_NOATIME code is very much designed to try O_NOATIME only _once_. Because even when the kernel supports O_NOATIME, if you have a shared object tree where you may not be the owner of all the files, a O_NOATIME open can fail with NOPERM ("You are not allowed to hide your accesses to this file").

This is why it uses that

    static unsigned int sha1_file_open_flag = O_NOATIME;

and if the O_NOATIME open ever fails (and the no-O_NOATIME open succeeds), it clears that flag. Exactly so that it will *not* end up in some kind of "let's open and fail and re-open" loop. It's designed to fail once.

Or at least that's how it used to be originally. This code has obviously changed since that early design. Now it seems to clear it for any non-ENOENT error. Which looks fine too.

          Linus