lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Tue, 2 Feb 2016 17:34:40 +0800
From:	Boqun Feng <boqun.feng@...il.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Paul McKenney <paulmck@...ux.vnet.ibm.com>,
	Will Deacon <will.deacon@....com>,
	Peter Zijlstra <peterz@...radead.org>,
	"Maciej W. Rozycki" <macro@...tec.com>,
	David Daney <ddaney@...iumnetworks.com>,
	Måns Rullgård <mans@...sr.com>,
	Ralf Baechle <ralf@...ux-mips.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [RFC][PATCH] mips: Fix arch_spin_unlock()

Hello Linus,

On Tue, Feb 02, 2016 at 12:19:04AM -0800, Linus Torvalds wrote:
> On Tue, Feb 2, 2016 at 12:07 AM, Linus Torvalds
> <torvalds@...ux-foundation.org> wrote:
> >
> > So we *absolutely* should say that *OF COURSE* these things work:
> >
> >  - CPU A:
> >
> >    .. initialize data structure -> smp_wmb() -> WRITE_ONCE(ptr);
> >
> >  - CPU B:
> >
> >    smp_load_acquire(ptr)  - we can rely on things behind "ptr" being initialized
> 
> That's a bad example, btw. I shouldn't have made it be a "pointer",
> because then we get the whole address dependency chain ordering
> anyway.
> 
> So instead of "ptr", read "state flag". It might just be an "int" that
> says "data has been initialized".
> 
> So
> 
>     .. initialize memory ..
>     smp_wmb();
>     WRITE_ONCE(&is_initialized, 1);
> 
> should pair with
> 
>     if (smp_load_acquire(&is_initialized))
>         ... we can read and write the data, knowing it has been initialized ..
> 
> exactly because "smp_wmb()" (cheap write barrier) might be cheaper
> than "smp_store_release()" (expensive full barrier) and thus
> preferred.
> 

Just to be clear, what Will, Paul and I are discussing here is about
local transitivity, which refers to something like this following
example:

(a, b and is_initialized are all initially zero)

P0:

	WRITE_ONCE(a, 1);
	smp_store_release(&is_initialized, 1);

P1:
	r1 = smp_load_acquire(&is_initialized);
	smp_store_release(&b, 1);


P2:
	r2 = smp_load_acquire(&b);
	r3 = READ_ONCE(a);

, in which case, r1 == 1 && r2 == 1 && r3 == 0 can not happen because
RELEASE+ACQUIRE pairs guarantee local transitivity.

More on local transitvity:

http://article.gmane.org/gmane.linux.kernel.virtualization/26856


And what I'm asking here is something like this following example:

(a, b and is_initialized are all initially zero)

P0:

	WRITE_ONCE(a, 1);
	smp_store_release(&is_initialized, 1);

P1:
	if (r1 = READ_ONCE(is_initialized))
		smp_store_release(&b, 1);


P2:
	if (r2 = READ_ONCE(b)) {
		smp_rmb();	
		r3 = READ_ONCE(a);
	}

, in which case, can r1 == 1 && r2 == 1 && r3 == 0 happen?


Please note this example is about two questions on local transitivity:

1.	Could "READ_ONCE(); if" extend a locally transitive chain.

2.	Could "READ_ONCE(); if; smp_rmb()" at least be a locally
	transitive chain termination?

> So mixing ordering metaphors actually does make sense, and should be
> entirely well-defined.
> 

I think Paul does agree that smp_{r,w}mb() with applicative memory
operations around could pair with smp_store_release() or
smp_load_acquire().

Hope I didn't misunderstand any of you or make you misunderstood with
each other..

Regards,
Boqun

> There's likely less reason to do it the other way (ie
> "smp_store_release()" on one side pairing with "LOAD_ONCE() +
> smp_rmb()" on the other) since there likely isn't the same kind of
> performance reason for that pairing. But even if we would never
> necessarily want to do it, I think our memory ordering rules would be
> *much* better for strongly stating that it has to work, than being
> timid and trying to make the rules weak.
> 
> Memory ordering is confusing enough as it is. We should not make
> people worry more than they already have to. Strong rules are good.
> 
>                     Linus

Download attachment "signature.asc" of type "application/pgp-signature" (474 bytes)

Powered by blists - more mailing lists