> Date: Sat, 17 Nov 2012 19:48:36 +0200
> From: Paul Irofti <p...@irofti.net>
>
> On Fri, Sep 21, 2012 at 12:15:51PM +0200, Mark Kettenis wrote:
> > > Date: Wed, 19 Sep 2012 15:41:09 +0300
> > > From: Paul Irofti <p...@irofti.net>
> > >
> > > I need this for acpi locking, okay?
> > >
> > > (I'd also like to get rid of the silly x86_* prefix to match the i386
> > > function name, but that's a different topic)
>
> Getting back to this...
>
> > I disagree with the off-topic bit. What we do need is a consistent
> > set of atomic operations across platforms. Haphazardly adding stuff
> > to match the local MD naming conventions isn't going to help.
>
> That project was shut-down. So I'll just skip this paragraph.
You give up easily don't you?
Even if Miod and Theo don't think we should start using more atomic
instructions in MI code, this doesn't mean we shouldn't try to define
a consistent set of them across platforms to use in MD code.
> > And there is another issue in this case. The i386 version does the
> > whole PCB_FAULT dance, whereas this version doesn't. So I think the
> >
> > #define atomic_ucas_32 ucas_32
> >
> > that was added (by you) is a mistake.
>
> Well, the current user for ucas_32/atomic_ucas_32 is the futex
> implementation in compat/linux. It's looking at an address from userland
> so it needs to do the PCB_FAULT dance.
Duh. I have a hard time seeing this used for anything else in the
forseeable future. Rename the function to futex_ucas32() or something.
> In the acpi case I don't think we need to. But if we do I'll add it.
It's wrong to use it in the acpi case. The recent addition of
stac/clac support makes that even more obvious.
> So I think we have the following options:
>
> 1/ Add a similar PCB aware op for amd64 for consistency.
No point doing that until we add Linux compat for amd64. And I don't
see any need for that.
> 2/ Split the i386 atomic_ucas_32 into two, one that's pcb aware and one
> that's not. And use the pcb aware one for the futex and the other one
> for the acpi locking. And of course the amd64 function in the diff
> bellow will match the non-pcb one from i386.
This is the way to go. You still need to come up with decent names
for these functions.
> > > Index: arch/amd64/include/atomic.h
> > > ===================================================================
> > > RCS file: /cvs/src/sys/arch/amd64/include/atomic.h,v
> > > retrieving revision 1.8
> > > diff -u -p -r1.8 atomic.h
> > > --- arch/amd64/include/atomic.h 23 Mar 2011 16:54:34 -0000 1.8
> > > +++ arch/amd64/include/atomic.h 19 Sep 2012 12:38:31 -0000
> > > @@ -90,6 +90,17 @@ x86_atomic_clearbits_u32(volatile u_int3
> > > __asm __volatile(LOCK " andl %1,%0" : "=m" (*ptr) : "ir" (~bits));
> > > }
> > >
> > > +static __inline int
> > > +x86_atomic_ucas_32(volatile uint32_t *ptr, uint32_t expect, uint32_t set)
> > > +{
> > > + u_long res;
> > > +
> > > + __asm volatile(LOCK " cmpxchgl %2, %1" : "=a" (res), "=m" (*ptr)
> > > + : "r" (set), "a" (expect), "m" (*ptr) : "memory");
> > > +
> > > + return (res);
> > > +}
> > > +
> > > static __inline u_long
> > > x86_atomic_cas_ul(volatile u_long *ptr, u_long expect, u_long set)
> > > {
