[PATCH] Unwire set/get_robust_list

[PATCH] Unwire set/get_robust_list

[Andreas Schwab]

The syscalls set/get_robust_list must not be wired up until
futex_atomic_cmpxchg_inatomic is implemented.  Otherwise the kernel will
hang in handle_futex_death.

Signed-off-by: Andreas Schwab <schwab@suse.de>

Index: linux-2.6.18-rc6/include/asm-powerpc/systbl.h
===================================================================
--- linux-2.6.18-rc6.orig/include/asm-powerpc/systbl.h	2006-09-04 18:12:43.000000000 +0200
+++ linux-2.6.18-rc6/include/asm-powerpc/systbl.h	2006-09-04 20:54:42.000000000 +0200
@@ -302,5 +302,5 @@ SYSCALL_SPU(symlinkat)
 SYSCALL_SPU(readlinkat)
 SYSCALL_SPU(fchmodat)
 SYSCALL_SPU(faccessat)
-COMPAT_SYS_SPU(get_robust_list)
-COMPAT_SYS_SPU(set_robust_list)
+SYSCALL(ni_syscall)
+SYSCALL(ni_syscall)
Index: linux-2.6.18-rc6/include/asm-powerpc/unistd.h
===================================================================
--- linux-2.6.18-rc6.orig/include/asm-powerpc/unistd.h	2006-09-04 18:12:43.000000000 +0200
+++ linux-2.6.18-rc6/include/asm-powerpc/unistd.h	2006-09-04 20:55:30.000000000 +0200
@@ -321,8 +321,8 @@
 #define __NR_readlinkat		296
 #define __NR_fchmodat		297
 #define __NR_faccessat		298
-#define __NR_get_robust_list	299
-#define __NR_set_robust_list	300
+/* Number 299 reserved for get_robust_list */
+/* Number 300 reserved for set_robust_list */
 
 #ifdef __KERNEL__
 

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Unwire set/get_robust_list

[David Woodhouse]

On Mon, 2006-09-04 at 21:27 +0200, Andreas Schwab wrote:
> The syscalls set/get_robust_list must not be wired up until
> futex_atomic_cmpxchg_inatomic is implemented.  Otherwise the kernel will
> hang in handle_futex_death.

Something like this? 

Untested-but-otherwise-Signed-off-by: David Woodhouse <dwmw2@infradead.org>

diff --git a/include/asm-powerpc/futex.h b/include/asm-powerpc/futex.h
index f1b3c00..e8ff12c 100644
--- a/include/asm-powerpc/futex.h
+++ b/include/asm-powerpc/futex.h
@@ -84,7 +84,30 @@ static inline int futex_atomic_op_inuser
 static inline int
 futex_atomic_cmpxchg_inatomic(int __user *uaddr, int oldval, int newval)
 {
-	return -ENOSYS;
+	int prev;
+
+        __asm__ __volatile__ (
+        LWSYNC_ON_SMP
+"1:     lwarx   %0,0,%2         # futex_atomic_cmpxchg_inatomic\n\
+        cmpw    0,%0,%3\n\
+        bne-    3f\n"
+        PPC405_ERR77(0,%2)
+"2:     stwcx.  %4,0,%2\n\
+        bne-    1b\n"
+        ISYNC_ON_SMP
+"3:	.section .fixup,\"ax\"\n\
+4:	li	%0,%5\n\
+	b	3b\n\
+	.previous\n\
+	.section __ex_table,\"a\"\n\
+	.align 3\n\
+	" PPC_LONG "1b,4b,2b,4b\n\
+	.previous" \
+        : "=&r" (prev), "+m" (*uaddr)
+        : "r" (uaddr), "r" (oldval), "r" (newval), "i" (-EFAULT)
+        : "cc", "memory");
+
+        return prev;
 }
 
 #endif /* __KERNEL__ */

-- 
dwmw2

Re: [PATCH] Unwire set/get_robust_list

[Andreas Schwab]

David Woodhouse <dwmw2@infradead.org> writes:

> On Mon, 2006-09-04 at 21:27 +0200, Andreas Schwab wrote:
>> The syscalls set/get_robust_list must not be wired up until
>> futex_atomic_cmpxchg_inatomic is implemented.  Otherwise the kernel will
>> hang in handle_futex_death.
>
> Something like this? 
>
> Untested-but-otherwise-Signed-off-by: David Woodhouse <dwmw2@infradead.org>

Thanks, sucessfully tested on 32bit (with the nptl testsuite of glibc).
Will do some 64bit testing tomorrow.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Unwire set/get_robust_list

[Paul Mackerras]

David Woodhouse writes:

> Something like this? 

Don't we need an access_ok() test in there?  Otherwise, looks fine to
me.

Thanks,
Paul.

Re: [PATCH] Unwire set/get_robust_list

[David Woodhouse]

On Tue, 2006-09-05 at 09:58 +1000, Paul Mackerras wrote:
> Don't we need an access_ok() test in there?  Otherwise, looks fine to
> me. 

Oops, good point. Like this then...

Do we actually need the memory clobber, btw? We're not using this for
locking in kernel space.

[PATCH] Implement PowerPC futex_atomic_cmpxchg_inatomic().

The sys_[gs]et_robust_list() syscalls were wired up on PowerPC but 
didn't work correctly because futex_atomic_cmpxchg_inatomic() wasn't
implemented. Implement it, based on __cmpxchg_u32()

Signed-off-by: David Woodhouse <dwmw2@infradead.org>

diff --git a/include/asm-powerpc/futex.h b/include/asm-powerpc/futex.h
index f1b3c00..936422e 100644
--- a/include/asm-powerpc/futex.h
+++ b/include/asm-powerpc/futex.h
@@ -84,7 +84,33 @@ static inline int futex_atomic_op_inuser
 static inline int
 futex_atomic_cmpxchg_inatomic(int __user *uaddr, int oldval, int newval)
 {
-	return -ENOSYS;
+	int prev;
+
+	if (!access_ok(VERIFY_WRITE, uaddr, sizeof(int)))
+		return -EFAULT;
+
+        __asm__ __volatile__ (
+        LWSYNC_ON_SMP
+"1:     lwarx   %0,0,%2         # futex_atomic_cmpxchg_inatomic\n\
+        cmpw    0,%0,%3\n\
+        bne-    3f\n"
+        PPC405_ERR77(0,%2)
+"2:     stwcx.  %4,0,%2\n\
+        bne-    1b\n"
+        ISYNC_ON_SMP
+"3:	.section .fixup,\"ax\"\n\
+4:	li	%0,%5\n\
+	b	3b\n\
+	.previous\n\
+	.section __ex_table,\"a\"\n\
+	.align 3\n\
+	" PPC_LONG "1b,4b,2b,4b\n\
+	.previous" \
+        : "=&r" (prev), "+m" (*uaddr)
+        : "r" (uaddr), "r" (oldval), "r" (newval), "i" (-EFAULT)
+        : "cc", "memory");
+
+        return prev;
 }
 
 #endif /* __KERNEL__ */

-- 
dwmw2

Re: [PATCH] Unwire set/get_robust_list

[David Woodhouse]

On Mon, 2006-09-04 at 21:27 +0200, Andreas Schwab wrote:
> The syscalls set/get_robust_list must not be wired up until
> futex_atomic_cmpxchg_inatomic is implemented.  Otherwise the kernel
> will hang in handle_futex_death. 

Either Andreas' or my patch ought to go into 2.6.18, since
[gs]et_robust_list() are otherwise broken there.

-- 
dwmw2

Re: [PATCH] Unwire set/get_robust_list

[Paul Mackerras]

Andreas Schwab writes:

> Thanks, sucessfully tested on 32bit (with the nptl testsuite of glibc).
> Will do some 64bit testing tomorrow.

Did you do that testing?  What was the result?

Could you test David's latest patch without the memory clobber?  For
reference, here it is.

Thanks,
Paul.

diff --git a/include/asm-powerpc/futex.h b/include/asm-powerpc/futex.h
index f1b3c00..936422e 100644
--- a/include/asm-powerpc/futex.h
+++ b/include/asm-powerpc/futex.h
@@ -84,7 +84,33 @@ static inline int futex_atomic_op_inuser
 static inline int
 futex_atomic_cmpxchg_inatomic(int __user *uaddr, int oldval, int newval)
 {
-	return -ENOSYS;
+	int prev;
+
+	if (!access_ok(VERIFY_WRITE, uaddr, sizeof(int)))
+		return -EFAULT;
+
+        __asm__ __volatile__ (
+        LWSYNC_ON_SMP
+"1:     lwarx   %0,0,%2         # futex_atomic_cmpxchg_inatomic\n\
+        cmpw    0,%0,%3\n\
+        bne-    3f\n"
+        PPC405_ERR77(0,%2)
+"2:     stwcx.  %4,0,%2\n\
+        bne-    1b\n"
+        ISYNC_ON_SMP
+"3:	.section .fixup,\"ax\"\n\
+4:	li	%0,%5\n\
+	b	3b\n\
+	.previous\n\
+	.section __ex_table,\"a\"\n\
+	.align 3\n\
+	" PPC_LONG "1b,4b,2b,4b\n\
+	.previous" \
+        : "=&r" (prev), "+m" (*uaddr)
+        : "r" (uaddr), "r" (oldval), "r" (newval), "i" (-EFAULT)
+        : "cc");
+
+        return prev;
 }
 
 #endif /* __KERNEL__ */

Re: [PATCH] Unwire set/get_robust_list

[Andreas Schwab]

Paul Mackerras <paulus@samba.org> writes:

> Andreas Schwab writes:
>
>> Thanks, sucessfully tested on 32bit (with the nptl testsuite of glibc).
>> Will do some 64bit testing tomorrow.
>
> Did you do that testing?  What was the result?

Yes, I did, successful.

> Could you test David's latest patch without the memory clobber?

I'll do.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Unwire set/get_robust_list

[Andreas Schwab]

Paul Mackerras <paulus@samba.org> writes:

> Could you test David's latest patch without the memory clobber?

Looks good (only tested on ppc64 kernel this time, but both 32 and 64 bit
userland).

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."