Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 31 Oct 2022 18:48:25 +0000
From:   Al Viro <viro@zeniv.linux.org.uk>
To:     Jann Horn <jannh@google.com>
Cc:     Linus Torvalds <torvalds@linux-foundation.org>,
        Miklos Szeredi <mszeredi@redhat.com>,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
        Will Deacon <will@kernel.org>
Subject: Re: [PATCH v2] fs: use acquire ordering in __fget_light()
Message-ID: <Y2AYecOnLTkhmZB1@ZenIV>
References: <20221031175256.2813280-1-jannh@google.com>
 <Y2APCmYNjYOYLf8G@ZenIV>
 <CAG48ez094n05c3QJMy7vZ5U=z87MzqYeKU97Na_R9O36_LJSXw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAG48ez094n05c3QJMy7vZ5U=z87MzqYeKU97Na_R9O36_LJSXw@mail.gmail.com>
Sender: Al Viro <viro@ftp.linux.org.uk>
Precedence: bulk

On Mon, Oct 31, 2022 at 07:13:30PM +0100, Jann Horn wrote:
> On Mon, Oct 31, 2022 at 7:08 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
> [...]
> > No arch-specific instances, so...
> > static __always_inline int
> > arch_atomic_read_acquire(const atomic_t *v)
> > {
> >         int ret;
> >
> >         if (__native_word(atomic_t)) {
> >                 ret = smp_load_acquire(&(v)->counter);
> >         } else {
> >                 ret = arch_atomic_read(v);
> >                 __atomic_acquire_fence();
> >         }
> >
> >         return ret;
> > }
> [...]
> > Do we really have any architectures where a structure with one
> > int field does *not* have a size that would satisfy that check?
> >
> > Is it future-proofing for masturbation sake, or am I missing something
> > real here?
> 
> include/linux/atomic/atomic-arch-fallback.h has a comment at the top that says:
> 
> // Generated by scripts/atomic/gen-atomic-fallback.sh
> // DO NOT MODIFY THIS FILE DIRECTLY

Hmm...  Apparently, the source is shared for atomic and atomic64, and the
check is intended for atomic64 counterpart of that thing on 32bit boxen.
Might make sense to put a comment in there...

The question about architectures with non-default implementations still
stands, though.

Anyway, it's unrelated to the patch itself.  I'm fine with it in the current
form.  Will apply for the next merge window, unless Linus wants it in right
now.