Re: implementing Futex

[Arnd Bergmann <arnd@arndb.de>]

On Thursday 13 August 2009, Michael Schnell wrote:
> I am planning to implement a Futex on the upcoming MMU-enabled NIOS
> architecture.

Ah, I'm always interested in new architectures. Are you already using
all the asm-generic header files that we have in 2.6.31? Please tell
me if you find problems with those.

> One of the tasks is to provide the appropriate atomic user-space
> operations for the Futex implementation. I was able to successfully do
> this for the x86 user space code and we have been discussing, how to do
>  this with other archs.
> 
> Now in the Kernel code in "linux-2.6/arch/x86/include/asm/futex.h" there
> are two different implementations for some atomic operations for the  Futex:
> 
> "futex_atomic_op_inuser()"    and
> "futex_atomic_cmpxhg__inatomic()"

Common trap. Just don't look at any x86 code when implementing
a new architecture. The code isn't necessarily bad, but much of
it has either too much compatibility crap for old interfaces, or
it optimizes some functionality much more than you'd care for.

In case of futex, look at how arch/sh does it. As Mike Frysinger
mentioned in http://lkml.org/lkml/2009/7/3/70 , the asm-generic
version is currently lacking functionality and we should use
the sh version for new stuff. 

Feel free to submit a patch that implements the operations correctly
for non-SMP architectures in include/asm-generic/futex.h.

> "futex_atomic_op_inuser()" seemingly accesses user space data while
> being run in Kernel space (doing the .section __ex_table trick).
> 
> Questions:
> 
> Is this correct ?

yes.
 
> In a non-SMP environment do we need to use really atomic code here, or
> does futex_atomic_op_inuser() run with interrupt disabled, anyway?

The sh version just disables interrupts to get atomicity.

> From reading the code (futex_atomic_op_inuser() seems only to be called
> once (in futex.c) ), it seems that futex_atomic_op_inuser() is not
> really used at all. It seems that it'd only called for futex_wake, and
> here the "Operation" is derived from the last parameter of the system
> call, which the man page says is ignored.  So, are all the operations
> implemented there really necessary or just "nice to have" ?

You misread futex_wake_op(). The operation comes from the second parameter
of the syscall, not the last one.

	Arnd <><