All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.ibm.com>
To: "H. Peter Anvin" <hpa@zytor.com>
Cc: Alexander Potapenko <glider@google.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: Fri, 29 Mar 2019 14:09:18 -0700	[thread overview]
Message-ID: <20190329210918.GZ4102@linux.ibm.com> (raw)
In-Reply-To: <8e32ab34-c14c-1ccb-76f9-0dcd729a0ef6@zytor.com>

On Fri, Mar 29, 2019 at 01:52:33PM -0700, H. Peter Anvin 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?
> 
> 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.

The atomic functions that return void don't need to order anything except
the input and output arguments.  The oddness with clear_bit() is that the
memory changed isn't necessarily the quantity referenced by the argument,
if the number of bits specified is large.

So (for example) atomic_inc() does not need a "memory" clobber, right?

							Thanx, Paul


  reply	other threads:[~2019-03-29 21:09 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 [this message]
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

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=20190329210918.GZ4102@linux.ibm.com \
    --to=paulmck@linux.ibm.com \
    --cc=dvyukov@google.com \
    --cc=glider@google.com \
    --cc=hpa@zytor.com \
    --cc=jyknight@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --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.