Messages in this thread | | | Date | Fri, 14 Nov 2014 08:58:20 -0800 | From | Alexander Duyck <> | 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
| |