linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: hpa@zytor.com
To: paulmck@linux.ibm.com, "Paul E. McKenney" <paulmck@linux.ibm.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 15:30:57 -0700	[thread overview]
Message-ID: <CD3717A9-E52B-425C-99B4-96ABD942E626@zytor.com> (raw)
In-Reply-To: <20190329220554.GD4102@linux.ibm.com>

On March 29, 2019 3:05:54 PM PDT, "Paul E. McKenney" <paulmck@linux.ibm.com> wrote:
>On Fri, Mar 29, 2019 at 02:51:26PM -0700, H. Peter Anvin wrote:
>> On 3/29/19 2:09 PM, Paul E. McKenney wrote:
>> >>
>> >> 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?
>> 
>> I don't believe that is true: the code calling it has a reasonable
>> expectation that previous memory operations have finished and later
>> memory operations have not started from the point of view of another
>> processor. You are more of an expert on memory ordering than I am,
>but
>> I'm 89% sure that there is plenty of code in the kernel which makes
>that
>> assumption.
>
>From Documentation/core-api/atomic_ops.rst:
>
>------------------------------------------------------------------------
>	void atomic_add(int i, atomic_t *v);
>	void atomic_sub(int i, atomic_t *v);
>	void atomic_inc(atomic_t *v);
>	void atomic_dec(atomic_t *v);
>
>These four routines add and subtract integral values to/from the given
>atomic_t value.  The first two routines pass explicit integers by
>which to make the adjustment, whereas the latter two use an implicit
>adjustment value of "1".
>
>One very important aspect of these two routines is that they DO NOT
>require any explicit memory barriers.  They need only perform the
>atomic_t counter update in an SMP safe manner.
>------------------------------------------------------------------------
>
>So, no, these functions do not imply any ordering other than to the
>variable modified.  This one predates my joining the Linux kernel
>community.  ;-)  So any cases where someone is relying on atomic_inc()
>to provide ordering are bugs.
>
>Now for value-returning atomics, for example, atomic_inc_return(),
>full ordering is indeed required.
>
>							Thanx, Paul

Ok.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

  reply	other threads:[~2019-03-29 22:31 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 [this message]
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=CD3717A9-E52B-425C-99B4-96ABD942E626@zytor.com \
    --to=hpa@zytor.com \
    --cc=dvyukov@google.com \
    --cc=glider@google.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).