linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Catalin Marinas <catalin.marinas@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Will Deacon <will.deacon@arm.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Oleg Nesterov <oleg@redhat.com>, Ingo Molnar <mingo@kernel.org>
Subject: Re: Q: schedule() and implied barriers on arm64
Date: Fri, 16 Oct 2015 10:28:11 -0700	[thread overview]
Message-ID: <20151016172811.GT3910@linux.vnet.ibm.com> (raw)
In-Reply-To: <CAHkRjk7z-bbTx0PyzMP0zqWxmQ4G7RrxydKQgrcigsnQEb0RBg@mail.gmail.com>

On Fri, Oct 16, 2015 at 05:55:35PM +0100, Catalin Marinas wrote:
> I'll try to reply in Will's absence, though I gave up trying to
> understand these threads long time ago ;).
> 
> On 16 October 2015 at 17:16, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Fri, Oct 16, 2015 at 09:04:22AM -0700, Paul E. McKenney wrote:
> >> On Fri, Oct 16, 2015 at 05:18:30PM +0200, Peter Zijlstra wrote:
> >> > If so, however, I suspect AARGH64 is borken and would need (just like
> >> > PPC):
> >> >
> >> > #define smp_mb__before_spinlock()   smp_mb()
> >> >
> >> > The problem is that schedule() (when a NO-OP) does:
> >> >
> >> >     smp_mb__before_spinlock();
> >> >     LOCK rq->lock
> >> >
> >> >     clear_bit()
> >> >
> >> >     UNLOCK rq->lock
> >> >
> >> > And nothing there implies a full barrier on AARGH64, since
> >> > smp_mb__before_spinlock() defaults to WMB, LOCK is an "ldaxr" or
> >> > load-acquire, UNLOCK is "stlrh" or store-release and clear_bit() isn't
> >> > anything.
> >> >
> >> > Pretty much every other arch has LOCK implying a full barrier, either
> >> > because its strongly ordered or because it needs one for the ACQUIRE
> >> > semantics.
> >>
> >> But I thought that it used a dmb in the spinlock code somewhere or
> >> another...
> >
> > arm does, arm64 not so much.
> 
> arm64 indeed does not have a dmb after spin_lock, it only has a
> load-acquire. So with the default smp_mb__before_spinlock() +
> spin_lock we have:
> 
>   smp_wmb()
>   loop
>     load-acquire
>     store
> 
> So (I think) this guarantees that any writes before wmb+lock would be
> visible before any reads _and_ writes after wmb+lock. However, the
> ordering with reads before wmb+lock is not guaranteed.

So RCU needs the following sort of guarantee:

	void task1(unsigned long flags)
	{
		WRITE_ONCE(x, 1);
		WRITE_ONCE(z, 1);
		raw_spin_unlock_irqrestore(&rnp->lock, flags);
	}

	void task2(unsigned long *flags)
	{
		raw_spin_lock_irqsave(&rnp->lock, *flags);
		smp_mb__after_unlock_lock();
		r1 = READ_ONCE(y);
		r2 = READ_ONCE(z);
	}

	void task3(void)
	{
		WRITE_ONCE(y, 1);
		smp_mb();
		r3 = READ_ONCE(x);
	}

	BUG_ON(!r1 && r2 && !r3); /* After the dust settles. */

In other words, if task2() acquires the lock after task1() releases it,
all CPUs must agree on the order of the operations in the two critical
sections, even if these other CPUs don't acquire the lock.

This same guarantee is needed if task1() and then task2() run in
succession on the same CPU with no additional synchronization of any sort.

Does this work on arm64?

							Thanx, Paul


  reply	other threads:[~2015-10-16 17:28 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-16 15:18 Q: schedule() and implied barriers on arm64 Peter Zijlstra
2015-10-16 16:04 ` Paul E. McKenney
2015-10-16 16:16   ` Peter Zijlstra
2015-10-16 16:28     ` Paul E. McKenney
2015-10-16 16:39       ` Peter Zijlstra
2015-10-16 16:55     ` Catalin Marinas
2015-10-16 17:28       ` Paul E. McKenney [this message]
2015-10-16 19:07         ` Peter Zijlstra
2015-10-16 19:20           ` Paul E. McKenney
2015-10-19 15:18         ` Catalin Marinas
2015-10-16 19:06       ` Peter Zijlstra
2015-10-19  7:06         ` Ingo Molnar
2015-10-19  9:04           ` Peter Zijlstra
2015-10-19 15:21           ` Catalin Marinas
2015-10-19 16:24             ` Peter Zijlstra
2015-10-20  8:37               ` Ingo Molnar
2015-10-27 16:19               ` Will Deacon
2015-10-27 18:40                 ` Peter Zijlstra
2015-10-28 10:39                   ` Will Deacon
2015-10-16 17:11   ` Catalin Marinas

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=20151016172811.GT3910@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=catalin.marinas@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=will.deacon@arm.com \
    /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).