svn commit: r300154 - head/sys/net

[Ian Lepore]

On Wed, 2016-05-18 at 17:35 +0000, Bjoern A. Zeeb wrote:
> > On 18 May 2016, at 17:32 , Ian Lepore <ian at freebsd.org> wrote:
> > 
> > On Wed, 2016-05-18 at 10:14 -0700, Nathan Whitehorn wrote:
> > > 
> > > On 05/18/16 08:50, Justin Hibbits wrote:
> > > > On Wed, 18 May 2016 15:45:12 +0000 (UTC)
> > > > Scott Long <scottl at FreeBSD.org> wrote:
> > > > 
> > > > > Author: scottl
> > > > > Date: Wed May 18 15:45:12 2016
> > > > > New Revision: 300154
> > > > > URL: https://svnweb.freebsd.org/changeset/base/300154
> > > > > 
> > > > > Log:
> > > > >   Activate the NO_64BIT_ATOMICS code for mips and powerpc
> > > > > 
> > > > > Modified:
> > > > >   head/sys/net/mp_ring.c
> > > > > 
> > > > > Modified: head/sys/net/mp_ring.c
> > > > > =============================================================
> > > > > ====
> > > > > =============
> > > > > --- head/sys/net/mp_ring.c	Wed May 18 15:44:45 2016
> > > > > (r300153) +++ head/sys/net/mp_ring.c	Wed May 18
> > > > > 15:45:12
> > > > > 2016	(r300154) @@ -37,15 +37,17 @@
> > > > > __FBSDID("$FreeBSD$");
> > > > >  #include <sys/malloc.h>
> > > > >  #include <machine/cpu.h>
> > > > > 
> > > > > -
> > > > > -
> > > > > -#include <net/mp_ring.h>
> > > > > +#if defined(__powerpc__) || defined(__mips__)
> > > > > +#define NO_64BIT_ATOMICS
> > > > > +#endif
> > > > > 
> > > > >  #if defined(__i386__)
> > > > >  #define atomic_cmpset_acq_64 atomic_cmpset_64
> > > > >  #define atomic_cmpset_rel_64 atomic_cmpset_64
> > > > >  #endif
> > > > > 
> > > > > +#include <net/mp_ring.h>
> > > > > +
> > > > >  union ring_state {
> > > > >  	struct {
> > > > >  		uint16_t pidx_head;
> > > > > 
> > > > powerpc64 defines both __powerpc__ and __powerpc64__, so you're
> > > > killing
> > > > atomics on powerpc64 with this.
> > > > 
> > > > - Justin
> > > > 
> > > 
> > > Don't all of our 64-bit platforms have 64-bit atomics? So you
> > > could
> > > just 
> > > #if defined(__LP64__) || defined(__i386__) || 
> > > defined(__whatever_the_thing_is_for_mips_n32__)
> > > -Nathan
> > > 
> > 
> > It may be more complicated than that, though.  armv6 can do 64-bit
> > atomics even tho it's 32-bit.  armv4, also 32-bit, can do 64-bit
> > atomics in the kernel but not in userland.
> > 
> > Maybe machine/atomic.h needs a #define that says whether 64-bit ops
> > are
> > available in the current compilation unit.  (And likewise for other
> > bit
> > sizes if we have arches that have other limitations.)
> 
> 
> Question because I didn’t follow the details, but how was this solved
> for the COUNTERS framework?
> 
> 
> /bz
> 

iirc, each platform implements counters its own way, probably the wrong
way on all of them except x86.

For some crazy reason the docs for COUNTERS say that it does not use
atomics.  I have no idea why the docs for an API are dictating
implementation, but I suspect it's because atomics are more expensive
on x86 than other alternatives.  So the arm code slavishly avoids using
atomics in COUNTERS even though doing so would be more effecient than
the current copied-from-x86 code.

-- Ian