All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexander Potapenko <glider@google.com>
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>
Subject: Re: Potentially missing "memory" clobbers in bitops.h for x86
Date: Mon, 1 Apr 2019 17:00:39 +0200	[thread overview]
Message-ID: <CAG_fn=UWGqg-U=CciYKOv11X6xQiQrQX5wKaoieE1co9v0rdBQ@mail.gmail.com> (raw)
In-Reply-To: <8e32ab34-c14c-1ccb-76f9-0dcd729a0ef6@zytor.com>

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ße, 33
80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

      parent reply	other threads:[~2019-04-01 15:00 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-28 14:14 Potentially missing "memory" clobbers in bitops.h for x86 Alexander Potapenko
2019-03-28 16:22 ` Paul E. McKenney
2019-03-29 15:54   ` Alexander Potapenko
2019-03-29 20:52     ` H. Peter Anvin
2019-03-29 21:09       ` Paul E. McKenney
2019-03-29 21:51         ` H. Peter Anvin
2019-03-29 22:05           ` Paul E. McKenney
2019-03-29 22:30             ` hpa
2019-04-01 10:53             ` Peter Zijlstra
2019-04-01 15:44               ` Paul E. McKenney
2019-04-01 16:04                 ` Peter Zijlstra
2019-04-01 15:00       ` Alexander Potapenko [this message]

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='CAG_fn=UWGqg-U=CciYKOv11X6xQiQrQX5wKaoieE1co9v0rdBQ@mail.gmail.com' \
    --to=glider@google.com \
    --cc=dvyukov@google.com \
    --cc=hpa@zytor.com \
    --cc=jyknight@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=paulmck@linux.ibm.com \
    --cc=peterz@infradead.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.