PI mutex support on ARM

PI mutex support on ARM

[Remy Bohmer]

[-- Attachment #1: Type: text/plain, Size: 466 bytes --]

Hello Thomas,

I have build an implementation for the routine
futex_atomic_cmpxchg_inatomic() to make PTHREAD_PRIO_INHERIT mutex
work on ARM.
Compared to other architectures where CPU specific assembler is used,
it is probably not the most optimal implementation possible, but I
tested it successfully on a Atmel AT91RM9200 core, and it could work
on other (UP) architectures as well.

What do you think of it?
Or do you know a better solution?

Kind Regards,

Remy

[-- Attachment #2: fix-userspace-prio-inherit.patch --]
[-- Type: text/x-patch, Size: 2039 bytes --]

Index: linux-2.6.21/include/asm/futex.h
===================================================================
--- linux-2.6.21.orig/include/asm/futex.h	2007-04-26 05:08:32.000000000 +0200
+++ linux-2.6.21/include/asm/futex.h	2007-07-06 15:11:01.000000000 +0200
@@ -1,6 +1,79 @@
-#ifndef _ASM_FUTEX_H
-#define _ASM_FUTEX_H
+#ifndef _ASM_ARM_FUTEX_H
+#define _ASM_ARM_FUTEX_H
 
-#include <asm-generic/futex.h>
+#ifdef __KERNEL__
+
+#include <linux/futex.h>
+#include <asm/errno.h>
+#include <asm/uaccess.h>
+
+static inline int
+futex_atomic_op_inuser (int encoded_op, int __user *uaddr)
+{
+	int op = (encoded_op >> 28) & 7;
+	int cmp = (encoded_op >> 24) & 15;
+	int oparg = (encoded_op << 8) >> 20;
+	int cmparg = (encoded_op << 20) >> 20;
+	int oldval = 0, ret;
+	if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
+		oparg = 1 << oparg;
+
+	if (! access_ok (VERIFY_WRITE, uaddr, sizeof(int)))
+		return -EFAULT;
+
+	pagefault_disable();
+
+	switch (op) {
+	case FUTEX_OP_SET:
+	case FUTEX_OP_ADD:
+	case FUTEX_OP_OR:
+	case FUTEX_OP_ANDN:
+	case FUTEX_OP_XOR:
+	default:
+		ret = -ENOSYS;
+	}
+
+	pagefault_enable();
+
+	if (!ret) {
+		switch (cmp) {
+		case FUTEX_OP_CMP_EQ: ret = (oldval == cmparg); break;
+		case FUTEX_OP_CMP_NE: ret = (oldval != cmparg); break;
+		case FUTEX_OP_CMP_LT: ret = (oldval < cmparg); break;
+		case FUTEX_OP_CMP_GE: ret = (oldval >= cmparg); break;
+		case FUTEX_OP_CMP_LE: ret = (oldval <= cmparg); break;
+		case FUTEX_OP_CMP_GT: ret = (oldval > cmparg); break;
+		default: ret = -ENOSYS;
+		}
+	}
+	return ret;
+}
+
+static inline int
+futex_atomic_cmpxchg_inatomic(int __user *uaddr, int oldval, int newval)
+{
+	int err = 0;
+	int uval;
+	unsigned long flags;
+
+	if (!access_ok(VERIFY_WRITE, uaddr, sizeof(int)))
+		return -EFAULT;
+
+	local_irq_save(flags);
+
+	err = get_user(uval, uaddr);
+	if (err)
+	{
+		local_irq_restore(flags);
+		 return -EFAULT;
+	}
+	if (uval == oldval)
+		err = put_user(newval, uaddr);
+
+	local_irq_restore(flags);
+
+	if (err) return -EFAULT;
+	return uval;
+}
 
 #endif

Re: PI mutex support on ARM

[Ingo Molnar]

* Remy Bohmer <l.pinguin@gmail.com> wrote:

> --- linux-2.6.21.orig/include/asm/futex.h	2007-04-26 05:08:32.000000000 +0200

patch format problem: never diff the asm/ files, diff the asm-arm/ 
files.

> +	local_irq_save(flags);
> +
> +	err = get_user(uval, uaddr);

is it safe to do a get_user() with irqs off?

> +	if (err)
> +	{
> +		local_irq_restore(flags);
> +		 return -EFAULT;
> +	}

(style problems. Use scripts/checkpatch.pl to verify your patches.)

> +	local_irq_restore(flags);

is this method of atomic ops SMP-safe?

	Ingo

Re: PI mutex support on ARM

[Remy Bohmer]

Hello Ingo,

> patch format problem: never diff the asm/ files, diff the asm-arm/ files.
Yeah, of course, I know, I believe I made a real mess of this one ...

> is it safe to do a get_user() with irqs off?
rhetorical question ;-))
I was wondering, Is it safe to use a normal spinlock here, like this?

------- 8-< -------------
	spin_lock(&futex_lock);

	err = get_user(uval, uaddr);
	if ((!err) && (uval == oldval))
		err = put_user(newval, uaddr);

	spin_unlock(&futex_lock);
------- 8-< -------------

> is this method of atomic ops SMP-safe?
another rhetorical question ;-))
Nope, I already knew that, that why I mentioned that it probably only
worked on UP...

Remy

Re: PI mutex support on ARM

[Arjan van de Ven]

On Sun, 2007-07-08 at 22:17 +0200, Remy Bohmer wrote:
> Hello Ingo,
> 
> > patch format problem: never diff the asm/ files, diff the asm-arm/ files.
> Yeah, of course, I know, I believe I made a real mess of this one ...
> 
> > is it safe to do a get_user() with irqs off?
> rhetorical question ;-))
> I was wondering, Is it safe to use a normal spinlock here, like this?

if get_user() isn't allowed with irq's off (and in general it's not,
maybe arm is special) then it also isn't allowed with spinlocks held..