2006-02-21 08:49:02

by Ingo Molnar

[permalink] [raw]
Subject: [patch 5/6] lightweight robust futexes: i386


i386: add the futex_atomic_cmpxchg_inuser() assembly implementation, and
wire up the new syscalls.

Signed-off-by: Ingo Molnar <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Signed-off-by: Arjan van de Ven <[email protected]>
Acked-by: Ulrich Drepper <[email protected]>

----

arch/i386/kernel/syscall_table.S | 2 ++
include/asm-i386/futex.h | 23 ++++++++++++++++++++++-
include/asm-i386/unistd.h | 4 +++-
3 files changed, 27 insertions(+), 2 deletions(-)

Index: linux/arch/i386/kernel/syscall_table.S
===================================================================
--- linux.orig/arch/i386/kernel/syscall_table.S
+++ linux/arch/i386/kernel/syscall_table.S
@@ -310,3 +310,5 @@ ENTRY(sys_call_table)
.long sys_pselect6
.long sys_ppoll
.long sys_unshare /* 310 */
+ .long sys_set_robust_list
+ .long sys_get_robust_list
Index: linux/include/asm-i386/futex.h
===================================================================
--- linux.orig/include/asm-i386/futex.h
+++ linux/include/asm-i386/futex.h
@@ -107,7 +107,28 @@ futex_atomic_op_inuser (int encoded_op,
static inline int
futex_atomic_cmpxchg_inuser(int __user *uaddr, int oldval, int newval)
{
- return -ENOSYS;
+ if (!access_ok(VERIFY_WRITE, uaddr, sizeof(int)))
+ return -EFAULT;
+
+ __asm__ __volatile__(
+ "1: " LOCK_PREFIX "cmpxchgl %3, %1 \n"
+
+ "2: .section .fixup, \"ax\" \n"
+ "3: mov %2, %0 \n"
+ " jmp 2b \n"
+ " .previous \n"
+
+ " .section __ex_table, \"a\" \n"
+ " .align 8 \n"
+ " .long 1b,3b \n"
+ " .previous \n"
+
+ : "=a" (oldval), "=m" (*uaddr)
+ : "i" (-EFAULT), "r" (newval), "0" (oldval)
+ : "memory"
+ );
+
+ return oldval;
}

#endif
Index: linux/include/asm-i386/unistd.h
===================================================================
--- linux.orig/include/asm-i386/unistd.h
+++ linux/include/asm-i386/unistd.h
@@ -316,8 +316,10 @@
#define __NR_pselect6 308
#define __NR_ppoll 309
#define __NR_unshare 310
+#define __NR_set_robust_list 311
+#define __NR_get_robust_list 312

-#define NR_syscalls 311
+#define NR_syscalls 313

/*
* user-visible error numbers are in the range -1 - -128: see


2006-02-21 15:57:42

by Chuck Ebbert

[permalink] [raw]
Subject: Re: [patch 5/6] lightweight robust futexes: i386

In-Reply-To: <[email protected]>

On Thu, 16 Feb 2006 at 10:42:34 +0100, Ingo Molnar wrote:

> --- linux-robust-list.q.orig/include/asm-i386/futex.h
> +++ linux-robust-list.q/include/asm-i386/futex.h
> @@ -107,7 +107,25 @@ futex_atomic_op_inuser (int encoded_op,
> static inline int
> futex_atomic_cmpxchg_inuser(int __user *uaddr, int oldval, int newval)
> {
> - return -ENOSYS;
> + __asm__ __volatile__(
> + "1: " LOCK_PREFIX "cmpxchgl %3, %1 \n"
> +
> + "2: .section .fixup, \"ax\" \n"
> + "3: mov %2, %0 \n"
> + " jmp 2b \n"
> + " .previous \n"
> +
> + " .section __ex_table, \"a\" \n"
> + " .align 8 \n"
> + " .long 1b,3b \n"
> + " .previous \n"
> +
> + : "=a" (oldval), "=m" (*uaddr)
^^^^
Should be "+m" because it's both read and written.

> + : "i" (-EFAULT), "r" (newval), "0" (oldval)
> + : "memory"
^^^^^^^^
Is this necessary? Every possible memory location that could be
affected has been listed in the operands if the above change is made.

> + );
> +
> + return oldval;
> }
>
> #endif
--
Chuck
"Equations are the Devil's sentences." --Stephen Colbert

2006-02-21 16:38:06

by Ulrich Drepper

[permalink] [raw]
Subject: Re: [patch 5/6] lightweight robust futexes: i386

On 2/21/06, Chuck Ebbert <[email protected]> wrote:
> > + : "=a" (oldval), "=m" (*uaddr)
> ^^^^
> Should be "+m" because it's both read and written.

No, this is why there is the "0" input parameter.


\> > + : "memory"
> ^^^^^^^^
> Is this necessary? Every possible memory location that could be
> affected has been listed in the operands if the above change is made.

This makes the asm a barrier for the compiler.

2006-02-22 21:51:56

by Chuck Ebbert

[permalink] [raw]
Subject: Re: [patch 5/6] lightweight robust futexes: i386

In-Reply-To: <[email protected]>

On Tue, 21 Feb 2006 at 08:38:04 -0800, Ulrich Drepper wrote:
> On 2/21/06, Chuck Ebbert <[email protected]> wrote:
> > > + : "=a" (oldval), "=m" (*uaddr)
> > ^^^^
> > Should be "+m" because it's both read and written.
>
> No, this is why there is the "0" input parameter.

But this is arg 1, not 0. With a memory clobber it's irrelevant
anyway though.

> > > + : "memory"
> > ^^^^^^^^
> > Is this necessary? Every possible memory location that could be
> > affected has been listed in the operands if the above change is made.
>
> This makes the asm a barrier for the compiler.

A comment to that effect would be nice; IOW document the reason for it.

--
Chuck
"Equations are the Devil's sentences." --Stephen Colbert