making asm-generic/futex.h completely usable

making asm-generic/futex.h completely usable

[Mike Frysinger]

ive been reading up on futexes of late and have been poking around the
futex kernel pieces to see what is needed to get Blackfin working with
it.

Blackfin falls into the "no hardware atomic instructions" category as
can easily be seen in our atomic.h: disable interrupts, do
load/stores, restore interrupts.  my understanding of the
futex_atomic_op_inuser() function is that this runs in process
context, so this same interrupt trick should work fine.  which leads
me to wonder why doesnt the asm-generic/futex.h header already take
this approach ?

seems to me that the SuperH implementation fits the bill nicely.
their arch/sh/include/asm/futex-irq.h looks like it could be literally
straight copied into asm-generic/futex.h thus making it fully
functional for everyone by default.

also, the futex_atomic_op_inuser() function itself seems to largely be
copy & paste between architectures ... the only differences are in the
atomic functions called by FUTEX_OP_XXX.  so perhaps the structure of
the header should follow that of asm-generic/uaccess.h in that arches
can override specific functions while still getting the rest for free
?
-mike

Re: making asm-generic/futex.h completely usable

[Arnd Bergmann]

On Friday 03 July 2009, Mike Frysinger wrote:
> ive been reading up on futexes of late and have been poking around the
> futex kernel pieces to see what is needed to get Blackfin working with
> it.
> 
> Blackfin falls into the "no hardware atomic instructions" category as
> can easily be seen in our atomic.h: disable interrupts, do
> load/stores, restore interrupts.  my understanding of the
> futex_atomic_op_inuser() function is that this runs in process
> context, so this same interrupt trick should work fine.  which leads
> me to wonder why doesnt the asm-generic/futex.h header already take
> this approach ?

The code does not run in user space, the name indicates that it
operates on variables in user space while running in the kernel.

The irq-disable trick fundamentally does not work on SMP systems,
which need architecture specific atomic operations for this.

> seems to me that the SuperH implementation fits the bill nicely.
> their arch/sh/include/asm/futex-irq.h looks like it could be literally
> straight copied into asm-generic/futex.h thus making it fully
> functional for everyone by default.

That sounds reasonable for the arch/sh/include/asm/futex.h file, but
the futex-irq.h file would still be reserved for non-SMP architectures.
I believe that out of the architectures currently using asm-generic/futex.h,
blackfin is the only one that supports SMP, so you still lose ;-)

On blackfin, being NOMMU, you could problably do a respective implementation
for SMP, along the lines of

/* include/asm-generic/futex-nommu-smp.h */
static inline int atomic_futex_op_xchg_set(int oparg, int __user *uaddr,
                                           int *oldval)
{
	int *addr = (__force void *)uaddr; /* only valid on NOMMU */
	if (!access_ok(VERIFY_WRITE, uaddr, 4))
		return -EFAULT;
	*oldval = xchg(addr, oparg);
        return 0;
}

static inline int atomic_futex_op_xchg_add(int oparg, int __user *uaddr,
                                           int *oldval)
{
	int *addr = (__force void *)uaddr; /* only valid on NOMMU */
	int tmp;
	if (!access_ok(VERIFY_WRITE, uaddr, 4))
		return -EFAULT;
	tmp = *addr;
	while ((*oldval = cmpxchg(addr, tmp, tmp + oparg)) != tmp)
		tmp = *oldval;
        return 0;
}

I.e. if your user space access is trivial, you can implement this using
the standard cmpxchg and xchg functions even on SMP.

	Arnd <><

Re: making asm-generic/futex.h completely usable

[Mike Frysinger]

On Fri, Jul 3, 2009 at 11:51, Arnd Bergmann wrote:
> On Friday 03 July 2009, Mike Frysinger wrote:
>> ive been reading up on futexes of late and have been poking around the
>> futex kernel pieces to see what is needed to get Blackfin working with
>> it.
>>
>> Blackfin falls into the "no hardware atomic instructions" category as
>> can easily be seen in our atomic.h: disable interrupts, do
>> load/stores, restore interrupts.  my understanding of the
>> futex_atomic_op_inuser() function is that this runs in process
>> context, so this same interrupt trick should work fine.  which leads
>> me to wonder why doesnt the asm-generic/futex.h header already take
>> this approach ?
>
> The code does not run in user space, the name indicates that it
> operates on variables in user space while running in the kernel.

when i said "process context" i didnt mean it was running in usermode.
 i meant the context of the kernel execution is of a process, so it
can sleep.  since it needs to use get/put user, that means it can
sleep fine.

> The irq-disable trick fundamentally does not work on SMP systems,
> which need architecture specific atomic operations for this.

i'm guessing spin lock with irq semantics wouldnt work due to the
possibility of get/put user sleeping ...

>> seems to me that the SuperH implementation fits the bill nicely.
>> their arch/sh/include/asm/futex-irq.h looks like it could be literally
>> straight copied into asm-generic/futex.h thus making it fully
>> functional for everyone by default.
>
> That sounds reasonable for the arch/sh/include/asm/futex.h file, but
> the futex-irq.h file would still be reserved for non-SMP architectures.
> I believe that out of the architectures currently using asm-generic/futex.h,
> blackfin is the only one that supports SMP, so you still lose ;-)

if the functions were able to be overridden on a per-function basis,
that is easy to handle with CONFIG_SMP in the Blackfin header.
-mike