linux-kernel - Re: [PATCH 1/3] arch: Introduce load_acquire() and store

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <546634AC.9070902@redhat.com>
Date:	Fri, 14 Nov 2014 08:58:20 -0800
From:	Alexander Duyck <alexander.h.duyck@...hat.com>
To:	David Laight <David.Laight@...LAB.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()


On 11/14/2014 02:45 AM, David Laight wrote:
> 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

Yeah, I think I might explore creating some lightweight barriers. The 
load/acquire stuff is a bit overkill for what is needed.

Thanks,

Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/