Re: [PATCH 1/3] arch: Introduce load_acquire() and store_release()

[Alexander Duyck]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/