Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <CAG_fn=X9tpSW6jvJd_COk_RU8zvcr-cxSWuJ05qcV0LotTxEEQ@mail.gmail.com>
 <20190328162222.GO4102@linux.ibm.com> <CAG_fn=WKA_4Vzsh6yH+=5TFv-EMrX87pr+2c_ANoH_MOVpbe1w@mail.gmail.com>
 <8e32ab34-c14c-1ccb-76f9-0dcd729a0ef6@zytor.com>
In-Reply-To: <8e32ab34-c14c-1ccb-76f9-0dcd729a0ef6@zytor.com>
From:   Alexander Potapenko <glider@google.com>
Date:   Mon, 1 Apr 2019 17:00:39 +0200
Message-ID: <CAG_fn=UWGqg-U=CciYKOv11X6xQiQrQX5wKaoieE1co9v0rdBQ@mail.gmail.com>
Subject: Re: Potentially missing "memory" clobbers in bitops.h for x86
To:     "H. Peter Anvin" <hpa@zytor.com>
Cc:     Paul McKenney <paulmck@linux.ibm.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Dmitriy Vyukov <dvyukov@google.com>,
        James Y Knight <jyknight@google.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Mar 29, 2019 at 9:52 PM H. Peter Anvin <hpa@zytor.com> wrote:
>
> On 3/29/19 8:54 AM, Alexander Potapenko wrote:
> >
> >> Of course, this would force the compiler to actually compute the
> >> offset, which would slow things down.  I have no idea whether this
> >> would be better or worse than just using the "memory" clobber.
> > Just adding the "memory" clobber to clear_bit() changes sizes of 5
> > kernel functions (the three mentioned above, plus hub_activate() and
> > native_send_call_func_ipi()) by a small margin.
> > This probably means the performance impact of this clobber is
> > negligible in this case.
>
> I would agree with that.
>
> Could you perhaps verify whether or not any of the above functions
> contains a currently manifest bug?
Yes, I've checked that none of the above functions contain the bug.
For a patch adding 7 memory constraints to various functions in
bitops.h there are already 258 functions that change their size.
I've skimmed through those with the biggest diffs and didn't find any
flaws, but that can be just pure luck.
I would expect such bugs to be more likely with full program
optimization kicking in.

> Note: the atomic versions of these functions obviously need to have
> "volatile" and the clobber anyway, as they are by definition barriers
> and moving memory operations around them would be a very serious error.
>
>         -hpa
>
>
>
--
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Stra=C3=9Fe, 33
80636 M=C3=BCnchen

Gesch=C3=A4ftsf=C3=BChrer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg