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  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:	Fri, 14 Nov 2014 10:45:41 +0000
From:	David Laight <David.Laight@...LAB.COM>
To:	'Alexander Duyck' <alexander.h.duyck@...hat.com>,
	"linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:	"mikey@...ling.org" <mikey@...ling.org>,
	"tony.luck@...el.com" <tony.luck@...el.com>,
	"mathieu.desnoyers@...ymtl.ca" <mathieu.desnoyers@...ymtl.ca>,
	"donald.c.skidmore@...el.com" <donald.c.skidmore@...el.com>,
	"peterz@...radead.org" <peterz@...radead.org>,
	"benh@...nel.crashing.org" <benh@...nel.crashing.org>,
	"heiko.carstens@...ibm.com" <heiko.carstens@...ibm.com>,
	"oleg@...hat.com" <oleg@...hat.com>,
	"will.deacon@....com" <will.deacon@....com>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"michael@...erman.id.au" <michael@...erman.id.au>,
	"matthew.vick@...el.com" <matthew.vick@...el.com>,
	"nic_swsd@...ltek.com" <nic_swsd@...ltek.com>,
	"geert@...ux-m68k.org" <geert@...ux-m68k.org>,
	"jeffrey.t.kirsher@...el.com" <jeffrey.t.kirsher@...el.com>,
	"fweisbec@...il.com" <fweisbec@...il.com>,
	"schwidefsky@...ibm.com" <schwidefsky@...ibm.com>,
	"linux@....linux.org.uk" <linux@....linux.org.uk>,
	"paulmck@...ux.vnet.ibm.com" <paulmck@...ux.vnet.ibm.com>,
	"torvalds@...ux-foundation.org" <torvalds@...ux-foundation.org>,
	"mingo@...nel.org" <mingo@...nel.org>
Subject: RE: [PATCH 1/3] arch: Introduce load_acquire() and store_release()

From: Alexander Duyck
> It is common for device drivers to make use of acquire/release semantics
> when dealing with descriptors stored in device memory.  On reviewing the
> documentation and code for smp_load_acquire() and smp_store_release() as
> well as reviewing an IBM website that goes over the use of PowerPC barriers
> at http://www.ibm.com/developerworks/systems/articles/powerpc.html it
> occurred to me that the same code could likely be applied to device drivers.
> 
> As a result this patch introduces load_acquire() and store_release().  The
> load_acquire() function can be used in the place of situations where a test
> for ownership must be followed by a memory barrier.  The below example is
> from ixgbe:
> 
> 	if (!rx_desc->wb.upper.status_error)
> 		break;
> 
> 	/* This memory barrier is needed to keep us from reading
> 	 * any other fields out of the rx_desc until we know the
> 	 * descriptor has been written back
> 	 */
> 	rmb();
> 
> With load_acquire() this can be changed to:
> 
> 	if (!load_acquire(&rx_desc->wb.upper.status_error))
> 		break;

If I'm quickly reading the 'new' code I need to look up yet another
function, with the 'old' code I can easily see the logic.

You've also added a memory barrier to the 'break' path - which isn't needed.

The driver might also have additional code that can be added before the barrier
so reducing the cost of the barrier.

The driver may also be able to perform multiple actions before a barrier is needed.

Hiding barriers isn't necessarily a good idea anyway.
If you are writing a driver you need to understand when and where they are needed.

Maybe you need a new (weaker) barrier to replace rmb() on some architectures.

...


	David

Powered by blists - more mailing lists