linuxsh-dev

Give a functional include/asm-sh/futex.h

[Kaz K. <kk...@rr...>]

The last problem is that the newer libc needs more complete futex
support in the kernel.  Now SH uses include/asm-generic/futex.h
which doesn't give the required functions.  The attached patch is
an obvious "last resort" implementation for the uni processor.
If the CPU has ll/sc insns, they can be used like as the other
architectures do.  I'll leave the llsc version to the folks who
can access such CPU.

Regards,
	kaz

Signed-off-by: Kaz Kojima <kk...@rr...>

--- GIT/linux-2.6/include/asm-sh/futex.h	2006-08-29 08:00:06.000000000 +0900
+++ linux-2.6.22-rc4/include/asm-sh/futex.h	2007-06-14 17:36:40.000000000 +0900
@@ -1,6 +1,101 @@
 #ifndef _ASM_FUTEX_H
 #define _ASM_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;
+	unsigned long flags;
+
+	if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
+		oparg = 1 << oparg;
+
+	if (!access_ok (VERIFY_WRITE, uaddr, sizeof(int)))
+		return -EFAULT;
+
+	pagefault_disable();
+
+	local_irq_save(flags);
+
+	switch (op) {
+	case FUTEX_OP_SET:
+		ret = get_user(oldval, uaddr);
+		if (!ret)
+			ret = put_user(oparg, uaddr);
+		break;
+	case FUTEX_OP_ADD:
+		ret = get_user(oldval, uaddr);
+		if (!ret)
+			ret = put_user(oldval + oparg, uaddr);
+		break;
+	case FUTEX_OP_OR:
+		ret = get_user(oldval, uaddr);
+		if (!ret)
+			ret = put_user(oldval | oparg, uaddr);
+		break;
+	case FUTEX_OP_ANDN:
+		ret = get_user(oldval, uaddr);
+		if (!ret)
+			ret = put_user(oldval & oparg, uaddr);
+		break;
+	case FUTEX_OP_XOR:
+		ret = get_user(oldval, uaddr);
+		if (!ret)
+			ret = put_user(oldval ^ oparg, uaddr);
+		break;
+	default:
+		ret = -ENOSYS;
+	}
+
+	local_irq_restore(flags);
+
+	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 ret, prev;
+	unsigned long flags;
+
+	if (!access_ok(VERIFY_WRITE, uaddr, sizeof(int)))
+		return -EFAULT;
+
+	local_irq_save(flags);
+
+	ret = get_user(prev, uaddr);
+	if (!ret && oldval == prev)
+		ret = put_user(newval, uaddr);
+
+	local_irq_restore(flags);
+
+	if (ret)
+		return ret;
+
+        return prev;
+}
+
+#endif /* __KERNEL__ */
 #endif

Re: Give a functional include/asm-sh/futex.h

[Paul M. <le...@li...>]

On Sun, Jun 17, 2007 at 07:27:05AM +0900, Kaz Kojima wrote:
> The last problem is that the newer libc needs more complete futex
> support in the kernel.  Now SH uses include/asm-generic/futex.h
> which doesn't give the required functions.  The attached patch is
> an obvious "last resort" implementation for the uni processor.
> If the CPU has ll/sc insns, they can be used like as the other
> architectures do.  I'll leave the llsc version to the folks who
> can access such CPU.
> 
Hmm.. this all looks terribly generic, if glibc requires the generic
implementation to be extended, perhaps this change should be made against
asm-generic/futex.h instead?

Re: Give a functional include/asm-sh/futex.h

[Paul M. <le...@li...>]

On Mon, Jun 18, 2007 at 10:11:47AM +0900, Kaz Kojima wrote:
> Paul Mundt <le...@li...> wrote:
> > Hmm.. this all looks terribly generic, if glibc requires the generic
> > implementation to be extended, perhaps this change should be made against
> > asm-generic/futex.h instead?
> 
> Perhaps if local_irq_* are removed, as I can't find any use of
> them in asm-generic/*.h.  It seems to me that the situation is
> quite similar to atomic.h.  So asm-sh/futex.h which includes
> asm-sh/futex-irq.h and asm-sh/futex-llsc.h selectively might
> be a way to go, though I have no strong opinion.
> 
Yes, but I'm rather concerned about the size, perhaps we should
and move it out-of-line and inline the op handlers?

If we just have a set of atomic_futex_op_xxx() ops that correspond to
each FUTEX_OP_xxx that are inlined in futex.h (FR-V seems to do this
already, in arch/frv/kernel/futex.c), futex_atomic_op_inuser() can be
brought out-of-line without any trouble, and we can bury all of the
local_irq_xxx() mess in the futex op itself.

Also, is futex_atomic_cmpxchg_inatomic() required, or should this simply
be a -ENOSYS if we don't have ll/sc methods? It seems like kernel/futex.c
handles the -ENOSYS case ok, and other platforms do return this way.

Re: Give a functional include/asm-sh/futex.h

[Paul M. <le...@li...>]

On Mon, Jun 18, 2007 at 11:34:30AM +0900, Kaz Kojima wrote:
> Paul Mundt <le...@li...> wrote:
> > Yes, but I'm rather concerned about the size, perhaps we should
> > and move it out-of-line and inline the op handlers?
> >
> > If we just have a set of atomic_futex_op_xxx() ops that correspond to
> > each FUTEX_OP_xxx that are inlined in futex.h (FR-V seems to do this
> > already, in arch/frv/kernel/futex.c), futex_atomic_op_inuser() can be
> > brought out-of-line without any trouble, and we can bury all of the
> > local_irq_xxx() mess in the futex op itself.
> 
> Looks not so different with atomic.h and I always prefer to follow
> the x86 way :-)  But, anyway, either way looks ok.
> 
How about this?

I think your FUTEX_OP_ANDN implementation was buggy, since you were doing
oldval & oparg instead of oldval & ~oparg..

--

 include/asm-sh/futex-irq.h |  111 +++++++++++++++++++++++++++++++++++++++++++++
 include/asm-sh/futex.h     |   79 ++++++++++++++++++++++++++++++--
 2 files changed, 186 insertions(+), 4 deletions(-)

diff --git a/include/asm-sh/futex.h b/include/asm-sh/futex.h
index 6a332a9..54d98b3 100644
--- a/include/asm-sh/futex.h
+++ b/include/asm-sh/futex.h
@@ -1,6 +1,77 @@
-#ifndef _ASM_FUTEX_H
-#define _ASM_FUTEX_H
+#ifndef __ASM_FUTEX_H
+#define __ASM_FUTEX_H
 
-#include <asm-generic/futex.h>
+#ifdef __KERNEL__
 
-#endif
+#include <linux/futex.h>
+#include <asm/errno.h>
+#include <asm/uaccess.h>
+
+/* XXX: UP variants, fix for SH-4A and SMP.. */
+#include <asm/futex-irq.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:
+		ret = atomic_futex_op_xchg_set(oparg, uaddr, &oldval);
+		break;
+	case FUTEX_OP_ADD:
+		ret = atomic_futex_op_xchg_add(oparg, uaddr, &oldval);
+		break;
+	case FUTEX_OP_OR:
+		ret = atomic_futex_op_xchg_or(oparg, uaddr, &oldval);
+		break;
+	case FUTEX_OP_ANDN:
+		ret = atomic_futex_op_xchg_and(~oparg, uaddr, &oldval);
+		break;
+	case FUTEX_OP_XOR:
+		ret = atomic_futex_op_xchg_xor(oparg, uaddr, &oldval);
+		break;
+	default:
+		ret = -ENOSYS;
+		break;
+	}
+
+	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)
+{
+	if (!access_ok(VERIFY_WRITE, uaddr, sizeof(int)))
+		return -EFAULT;
+
+	return atomic_futex_op_cmpxchg_inatomic(uaddr, oldval, newval);
+}
+
+#endif /* __KERNEL__ */
+#endif /* __ASM_SH_FUTEX_H */
diff --git a/dev/null b/include/asm-sh/futex-irq.h
new file mode 100644
index 0000000..a9f16a7
--- /dev/null
+++ b/include/asm-sh/futex-irq.h
@@ -0,0 +1,111 @@
+#ifndef __ASM_SH_FUTEX_IRQ_H
+#define __ASM_SH_FUTEX_IRQ_H
+
+#include <asm/system.h>
+
+static inline int atomic_futex_op_xchg_set(int oparg, int __user *uaddr,
+					   int *oldval)
+{
+	unsigned long flags;
+	int ret;
+
+	local_irq_save(flags);
+
+	ret = get_user(*oldval, uaddr);
+	if (!ret)
+		ret = put_user(oparg, uaddr);
+
+	local_irq_restore(flags);
+
+	return ret;
+}
+
+static inline int atomic_futex_op_xchg_add(int oparg, int __user *uaddr,
+					   int *oldval)
+{
+	unsigned long flags;
+	int ret;
+
+	local_irq_save(flags);
+
+	ret = get_user(*oldval, uaddr);
+	if (!ret)
+		ret = put_user(*oldval + oparg, uaddr);
+
+	local_irq_restore(flags);
+
+	return ret;
+}
+
+static inline int atomic_futex_op_xchg_or(int oparg, int __user *uaddr,
+					  int *oldval)
+{
+	unsigned long flags;
+	int ret;
+
+	local_irq_save(flags);
+
+	ret = get_user(*oldval, uaddr);
+	if (!ret)
+		ret = put_user(*oldval | oparg, uaddr);
+
+	local_irq_restore(flags);
+
+	return ret;
+}
+
+static inline int atomic_futex_op_xchg_and(int oparg, int __user *uaddr,
+					   int *oldval)
+{
+	unsigned long flags;
+	int ret;
+
+	local_irq_save(flags);
+
+	ret = get_user(*oldval, uaddr);
+	if (!ret)
+		ret = put_user(*oldval & oparg, uaddr);
+
+	local_irq_restore(flags);
+
+	return ret;
+}
+
+static inline int atomic_futex_op_xchg_xor(int oparg, int __user *uaddr,
+					   int *oldval)
+{
+	unsigned long flags;
+	int ret;
+
+	local_irq_save(flags);
+
+	ret = get_user(*oldval, uaddr);
+	if (!ret)
+		ret = put_user(*oldval ^ oparg, uaddr);
+
+	local_irq_restore(flags);
+
+	return ret;
+}
+
+static inline int atomic_futex_op_cmpxchg_inatomic(int __user *uaddr,
+						   int oldval, int newval)
+{
+	unsigned long flags;
+	int ret, prev = 0;
+
+	local_irq_save(flags);
+
+	ret = get_user(prev, uaddr);
+	if (!ret && oldval == prev)
+		ret = put_user(newval, uaddr);
+
+	local_irq_restore(flags);
+
+	if (ret)
+		return ret;
+
+	return prev;
+}
+
+#endif /* __ASM_SH_FUTEX_IRQ_H */

Re: [linux] Re: Give a functional include/asm-sh/futex.h

[Kaz K. <kk...@rr...>]

Paul Mundt <le...@li...> wrote:
> How about this?
> 
> I think your FUTEX_OP_ANDN implementation was buggy, since you were doing
> oldval & oparg instead of oldval & ~oparg..

Looks fine.  Thanks!

Regards,
	kaz

Re: Give a functional include/asm-sh/futex.h

[Kaz K. <kk...@rr...>]

Paul Mundt <le...@li...> wrote:
> Yes, but I'm rather concerned about the size, perhaps we should
> and move it out-of-line and inline the op handlers?
>
> If we just have a set of atomic_futex_op_xxx() ops that correspond to
> each FUTEX_OP_xxx that are inlined in futex.h (FR-V seems to do this
> already, in arch/frv/kernel/futex.c), futex_atomic_op_inuser() can be
> brought out-of-line without any trouble, and we can bury all of the
> local_irq_xxx() mess in the futex op itself.

Looks not so different with atomic.h and I always prefer to follow
the x86 way :-)  But, anyway, either way looks ok.

> Also, is futex_atomic_cmpxchg_inatomic() required, or should this simply
> be a -ENOSYS if we don't have ll/sc methods? It seems like kernel/futex.c
> handles the -ENOSYS case ok, and other platforms do return this way.

Former, I believe.  It looks that kernel simply ignore such platforms
in full futex support which is required by glibc.  In general, platforms
without ll/sc-like can't use nptl and aren't target of current glibc in
the first place.  SH is an exception in this regard by virtue of gUSA.

Regards,
	kaz

Re: Give a functional include/asm-sh/futex.h

[Kaz K. <kk...@rr...>]

Paul Mundt <le...@li...> wrote:
> Hmm.. this all looks terribly generic, if glibc requires the generic
> implementation to be extended, perhaps this change should be made against
> asm-generic/futex.h instead?

Perhaps if local_irq_* are removed, as I can't find any use of
them in asm-generic/*.h.  It seems to me that the situation is
quite similar to atomic.h.  So asm-sh/futex.h which includes
asm-sh/futex-irq.h and asm-sh/futex-llsc.h selectively might
be a way to go, though I have no strong opinion.

Regards,
	kaz