[Xenomai-core] [0/9] Posix skin user-space mutexes

[Xenomai-core] [0/9] Posix skin user-space mutexes

[Gilles Chanteperdrix]

Hi,

the patch series to come for review adds support for user-space mutexes to the
posix skin. Since I wanted this support to be available on my AT91RM9200, the
patch series start with patches which are mainly for the ARM architecture, to
end with reimplementation of the kernel-space and user-space mutex services for
the posix skin.

Some patches brought some questions when making them, questions which will be
discussed in the mail accompanying the patch.

Since I do not use quilt, some patches will inevitably mix several
modifications, especially the patch to ksrc/skins/posix/syscall.c. I promise,
next time I will use quilt.

The result has only be tested in the fast (no syscall) case, to evaluate its
performance, I will start tests trying to cover the syscall case now, and keep
you informed.

Thanks in advance for your review.

-- 


					    Gilles.

[Xenomai-core] [1/9] Support for non cached memory mappings

[Gilles Chanteperdrix]

This patch adds architecture independent support for non cached memory
mappings. This is necessary on ARM architecture with VIVT cache to share a
mapping between kernel and user-space, but may be used in other situations (who
knows).

---
 include/asm-generic/wrappers.h |   12 ++++++++----
 include/native/heap.h          |    1 +
 include/nucleus/heap.h         |    2 ++
 ksrc/nucleus/heap.c            |   23 ++++++++++++++++++-----
 ksrc/skins/native/heap.c       |    8 +++++++-
 5 files changed, 36 insertions(+), 10 deletions(-)

Index: include/native/heap.h
===================================================================
--- include/native/heap.h	(revision 3718)
+++ include/native/heap.h	(working copy)
@@ -33,6 +33,7 @@
 #define H_MAPPABLE 0x200	/* Memory is mappable to user-space. */
 #define H_SINGLE   0x400	/* Manage as single-block area. */
 #define H_SHARED   (H_MAPPABLE|H_SINGLE) /* I.e. shared memory segment. */
+#define H_NONCACHED 0x800
 
 /** Structure containing heap-information useful to users.
  *
Index: include/asm-generic/wrappers.h
===================================================================
--- include/asm-generic/wrappers.h	(revision 3718)
+++ include/asm-generic/wrappers.h	(working copy)
@@ -62,7 +62,7 @@ unsigned long __va_to_kva(unsigned long 
 
 #define wrap_remap_vm_page(vma,from,to) ({ \
     vma->vm_flags |= VM_RESERVED; \
-    remap_page_range(from,virt_to_phys((void *)__va_to_kva(to)),PAGE_SIZE,PAGE_SHARED); \
+    remap_page_range(from,virt_to_phys((void *)__va_to_kva(to)),PAGE_SIZE,vma->vm_page_prot); \
 })
 #define wrap_remap_io_page_range(vma,from,to,size,prot) ({ \
     vma->vm_flags |= VM_RESERVED; \
@@ -223,7 +223,7 @@ unsigned long __va_to_kva(unsigned long 
  * memory. Anyway, this legacy would only hit setups using pre-2.6.11
  * kernel revisions. */
 #define wrap_remap_vm_page(vma,from,to) \
-    remap_pfn_range(vma,from,virt_to_phys((void *)__va_to_kva(to)) >> PAGE_SHIFT,PAGE_SHIFT,PAGE_SHARED)
+    remap_pfn_range(vma,from,virt_to_phys((void *)__va_to_kva(to)) >> PAGE_SHIFT,PAGE_SHIFT,vma->vm_page_prot)
 #define wrap_remap_io_page_range(vma,from,to,size,prot)  ({		\
     (vma)->vm_page_prot = pgprot_noncached((vma)->vm_page_prot);	\
     /* Sets VM_RESERVED | VM_IO | VM_PFNMAP on the vma. */		\
@@ -236,7 +236,7 @@ unsigned long __va_to_kva(unsigned long 
 #else /* LINUX_VERSION_CODE < KERNEL_VERSION(2,6,10) */
 #define wrap_remap_vm_page(vma,from,to) ({ \
     vma->vm_flags |= VM_RESERVED; \
-    remap_page_range(from,virt_to_phys((void *)__va_to_kva(to)),PAGE_SIZE,PAGE_SHARED); \
+    remap_page_range(from,virt_to_phys((void *)__va_to_kva(to)),PAGE_SIZE,vma->vm_page_prot); \
 })
 #define wrap_remap_io_page_range(vma,from,to,size,prot) ({	\
       vma->vm_flags |= VM_RESERVED;				\
@@ -248,7 +248,11 @@ unsigned long __va_to_kva(unsigned long 
     })
 #endif /* LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,15) */
 
-#define wrap_switch_mm(prev,next,task)	\
+#ifndef __GFP_BITS_SHIFT
+#define __GFP_BITS_SHIFT 20
+#endif
+
+#define wrap_switch_mm(prev,next,task)		\
     switch_mm(prev,next,task)
 #define wrap_enter_lazy_tlb(mm,task)	\
     enter_lazy_tlb(mm,task)
Index: include/nucleus/heap.h
===================================================================
--- include/nucleus/heap.h	(revision 3718)
+++ include/nucleus/heap.h	(working copy)
@@ -57,6 +57,8 @@
 #define XNHEAP_PCONT   1
 #define XNHEAP_PLIST   2
 
+#define XNHEAP_GFP_NONCACHED (1 << __GFP_BITS_SHIFT)
+
 typedef struct xnextent {
 
 	xnholder_t link;
Index: ksrc/skins/native/heap.c
===================================================================
--- ksrc/skins/native/heap.c	(revision 3718)
+++ ksrc/skins/native/heap.c	(working copy)
@@ -205,6 +205,10 @@ static void __heap_flush_private(xnheap_
  * operations with I/O devices. The physical address of the
  * heap can be obtained by a call to rt_heap_inquire().
  *
+ * - H_NONCACHED causes the heap not to be cached. This is necessary on
+ * platforms such as ARM to share a heap between kernel and user-space.
+ * Note that this flag is not compatible with the H_DMA flag.
+ *
  * @return 0 is returned upon success. Otherwise:
  *
  * - -EEXIST is returned if the @a name is already in use by some
@@ -260,7 +264,9 @@ int rt_heap_create(RT_HEAP *heap, const 
 
 		err = xnheap_init_mapped(&heap->heap_base,
 					 heapsize,
-					 (mode & H_DMA) ? GFP_DMA : 0);
+					 ((mode & H_DMA) ? GFP_DMA : 0)
+					 | ((mode & H_NONCACHED) ?
+					    XNHEAP_GFP_NONCACHED : 0));
 		if (err)
 			return err;
 
Index: ksrc/nucleus/heap.c
===================================================================
--- ksrc/nucleus/heap.c	(revision 3718)
+++ ksrc/nucleus/heap.c	(working copy)
@@ -1097,9 +1097,13 @@ static int xnheap_mmap(struct file *file
 
 	vaddr = (unsigned long)heap->archdep.heapbase;
 
-	if (!heap->archdep.kmflags) {
+	if (!heap->archdep.kmflags
+	    || heap->archdep.kmflags == XNHEAP_GFP_NONCACHED) {
 		unsigned long maddr = vma->vm_start;
 
+		if (heap->archdep.kmflags == XNHEAP_GFP_NONCACHED)
+			vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
+
 		while (size > 0) {
 			if (xnarch_remap_vm_page(vma, maddr, vaddr))
 				return -EAGAIN;
@@ -1174,9 +1178,13 @@ static inline void *__alloc_and_reserve_
 
 	/* Size must be page-aligned. */
 
-	if (!kmflags) {
-		ptr = vmalloc(size);
-
+	if (!kmflags || kmflags == XNHEAP_GFP_NONCACHED) {
+		if (!kmflags)
+			ptr = vmalloc(size);
+		else
+			ptr = __vmalloc(size,
+					GFP_KERNEL | __GFP_HIGHMEM,
+					pgprot_noncached(PAGE_KERNEL));
 		if (!ptr)
 			return NULL;
 
@@ -1217,7 +1225,7 @@ static inline void __unreserve_and_free_
 
 	vabase = (unsigned long)ptr;
 
-	if (!kmflags) {
+	if (!kmflags  || kmflags == XNHEAP_GFP_NONCACHED) {
 		for (vaddr = vabase; vaddr < vabase + size; vaddr += PAGE_SIZE)
 			ClearPageReserved(virt_to_page(__va_to_kva(vaddr)));
 
@@ -1241,6 +1249,11 @@ int xnheap_init_mapped(xnheap_t *heap, u
 
 	/* Caller must have accounted for internal overhead. */
 	heapsize = xnheap_align(heapsize, PAGE_SIZE);
+
+	if ((memflags & XNHEAP_GFP_NONCACHED)
+	    && memflags != XNHEAP_GFP_NONCACHED)
+		return -EINVAL;
+
 	heapbase = __alloc_and_reserve_heap(heapsize, memflags);
 
 	if (!heapbase)

-- 


					    Gilles.

[Xenomai-core] [2/9] Define XNARCH_SHARED_HEAP_FLAGS

[Gilles Chanteperdrix]

This patch defines the macro XNARCH_SHARED_HEAP_FLAGS to be set to
XNHEAP_GFP_NONCACHED on ARM with VIVT cache. I assumed that ARM with VIPT cache
would not need non-cached mappings when sharing memory between kernel and
user-space. Please correct me if I am wrong.

---
 asm-arm/hal.h        |    3 ++-
 asm-generic/system.h |    6 ++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

Index: include/asm-generic/system.h
===================================================================
--- include/asm-generic/system.h	(revision 3718)
+++ include/asm-generic/system.h	(working copy)
@@ -238,6 +238,12 @@ static inline int xnlock_dbg_release(xnl
 #define xnarch_logerr(fmt, args...)	printk(KERN_ERR XNARCH_PROMPT fmt, ##args)
 #define xnarch_printf(fmt, args...)	printk(KERN_INFO XNARCH_PROMPT fmt, ##args)
 
+#ifndef RTHAL_SHARED_HEAP_FLAGS
+#define XNARCH_SHARED_HEAP_FLAGS 0
+#else /* !RTHAL_SHARED_HEAP_FLAGS */
+#define XNARCH_SHARED_HEAP_FLAGS RTHAL_SHARED_HEAP_FLAGS
+#endif /* !RTHAL_SHARED_HEAP_FLAGS */
+
 typedef cpumask_t xnarch_cpumask_t;
 
 #ifdef CONFIG_SMP
Index: include/asm-arm/hal.h
===================================================================
--- include/asm-arm/hal.h	(revision 3718)
+++ include/asm-arm/hal.h	(working copy)
@@ -108,10 +108,11 @@ static inline __attribute_const__ unsign
 #include <asm/processor.h>
 #include <asm/ipipe.h>
 #include <asm/mach/irq.h>
+#include <asm/cacheflush.h>
 
 #define RTHAL_TIMER_IRQ   __ipipe_mach_timerint
 
-#define RTHAL_SHARED_HEAP_FLAGS XNHEAP_GFP_NONCACHED
+#define RTHAL_SHARED_HEAP_FLAGS (cache_is_vivt() ? XNHEAP_GFP_NONCACHED : 0)
 
 #define rthal_grab_control()     do { } while(0)
 #define rthal_release_control()  do { } while(0)


-- 


					    Gilles.

[Xenomai-core] [3/9] Define more atomic operations in user-space

[Gilles Chanteperdrix]

This patch implements the _read, _set, and _cmpxchg operations on atomic_long_t
and atomic_ptr_t in user-space in include/asm-generic/atomic.h which should be
included at the end of include/asm-*/atomic.h after the definition of the same
operations for the atomic_t type and atomic64_t type on 64 bits platforms.

These operations are the basic operations used by user-space mutexes. Maybe we
should add the xnarch_ prefix ?

---
 Makefile.am |    2 -
 atomic.h    |   68 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 1 deletion(-)

Index: include/asm-generic/atomic.h
===================================================================
--- include/asm-generic/atomic.h	(revision 0)
+++ include/asm-generic/atomic.h	(revision 0)
@@ -0,0 +1,68 @@
+#ifndef ATOMIC_H
+#define ATOMIC_H
+
+#ifndef __KERNEL__
+#include <bits/wordsize.h>
+#if __WORDSIZE == 64
+typedef atomic64_t atomic_long_t;
+
+static inline long atomic_long_read(atomic_long_t *l)
+{
+        atomic64_t *v = (atomic64_t *)l;
+
+        return (long)atomic64_read(v);
+}
+
+static inline void atomic_long_set(atomic_long_t *l, long i)
+{
+        atomic64_t *v = (atomic64_t *)l;
+
+        atomic64_set(v, i);
+}
+
+#define atomic_long_cmpxchg(l, old, new) \
+        (atomic64_cmpxchg((atomic64_t *)(l), (old), (new)))
+
+#else /* __WORDSIZE == 32 */
+typedef atomic_t atomic_long_t;
+
+static inline long atomic_long_read(atomic_long_t *l)
+{
+        atomic_t *v = (atomic_t *)l;
+
+        return (long)atomic_read(v);
+}
+
+static inline void atomic_long_set(atomic_long_t *l, long i)
+{
+        atomic_t *v = (atomic_t *)l;
+
+        atomic_set(v, i);
+}
+
+#define atomic_long_cmpxchg(l, old, new) \
+        (atomic_cmpxchg((atomic_t *)(l), (old), (new)))
+#endif /* __WORDSIZE == 32 */
+#endif /* __KERNEL__ */
+
+typedef atomic_long_t atomic_ptr_t;
+
+static inline void *atomic_ptr_read(atomic_ptr_t *l)
+{
+        atomic_long_t *v = (atomic_long_t *)l;
+
+        return (void *)atomic_long_read(v);
+}
+
+static inline void atomic_ptr_set(atomic_ptr_t *l, void *i)
+{
+        atomic_long_t *v = (atomic_long_t *)l;
+
+        atomic_long_set(v, (long)i);
+}
+
+#define atomic_ptr_cmpxchg(l, old, new) \
+        (void *)(atomic_long_cmpxchg((atomic_long_t *)(l), \
+                                     (long)(old), (long)(new)))
+
+#endif /* ATOMIC_H */
Index: include/asm-generic/Makefile.am
===================================================================
--- include/asm-generic/Makefile.am	(revision 3718)
+++ include/asm-generic/Makefile.am	(working copy)
@@ -1,5 +1,5 @@
 includesubdir = $(includedir)/asm-generic
 
-includesub_HEADERS = arith.h features.h hal.h syscall.h system.h wrappers.h
+includesub_HEADERS = arith.h features.h hal.h syscall.h system.h wrappers.h atomic.h
 
 SUBDIRS = bits

-- 


					    Gilles.

[Xenomai-core] [4/9] Define ARM atomic operations in user-space

[Gilles Chanteperdrix]

This patch implements the _read, _set, and _cmpxchg operations on the atomic_t
type in user-space for ARM, using ldrex/strex on ARM v6, and the Linux kernel
helper kuser_cmpxchg on ARM pre-v6 (so, without syscalls) without SMP. Only the
SMP case for pre-v6 ARMs still use syscalls, but this case should not be so
frequent anyway.

A new macro XNARCH_HAVE_US_ATOMIC_CMPXCHG is defined both in kernel-space and
user-space, so that the kernel-space can rely on the user-space having
atomic_cmpxchg defined.

The plan is to implement these atomic operations for other platforms, defining
the XNARCH_HAVE_US_ATOMIC_CMPXCHG macro when this is done. When all platforms
define XNARCH_HAVE_US_ATOMIC_CMPXCGH, we will be able to remove all the
#ifdefs.

A question arise about memory barriers. I assumed that atomic_cmpxchg implied a
barrier, but I am not to sure about that. If that is not the case, we will have
to implement barriers in user-space as well, and call the in the apropriate
places in mutexes implementation.

---
 atomic.h |  169 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 166 insertions(+), 3 deletions(-)

Index: include/asm-arm/atomic.h
===================================================================
--- include/asm-arm/atomic.h	(revision 3718)
+++ include/asm-arm/atomic.h	(working copy)
@@ -23,7 +23,6 @@
 #ifndef _XENO_ASM_ARM_ATOMIC_H
 #define _XENO_ASM_ARM_ATOMIC_H
 
-
 #ifdef __KERNEL__
 
 #include <linux/bitops.h>
@@ -35,6 +34,8 @@
 #define xnarch_memory_barrier()  	smp_mb()
 
 #if __LINUX_ARM_ARCH__ >= 6
+#define XNARCH_HAVE_US_ATOMIC_CMPXCHG
+
 static inline void atomic_set_mask(unsigned long mask, unsigned long *addr)
 {
     unsigned long tmp, tmp2;
@@ -58,6 +59,11 @@ static inline void atomic_set_mask(unsig
     *addr |= mask;
     local_irq_restore_hw(flags);
 }
+
+#ifndef CONFIG_SMP
+#define XNARCH_HAVE_US_ATOMIC_CMPXCHG
+#endif /* CONFIG_SMP */
+
 #endif /* ARM_ARCH_6 */
 
 #define xnarch_atomic_set(pcounter,i)          atomic_set(pcounter,i)
@@ -75,9 +81,14 @@ typedef atomic_t atomic_counter_t;
 
 #include <asm/xenomai/features.h>
 #include <asm/xenomai/syscall.h>
+#include <nucleus/compiler.h>
 
 typedef struct { volatile int counter; } atomic_counter_t;
 
+typedef atomic_counter_t atomic_t;
+
+#define atomic_read(v)	((v)->counter)
+
 /*
  * This function doesn't exist, so you'll get a linker error
  * if something tries to do an invalid xchg().
@@ -129,6 +140,40 @@ __xchg(volatile void *ptr, unsigned long
  * Atomic operations lifted from linux/include/asm-arm/atomic.h 
  */
 #if CONFIG_XENO_ARM_ARCH >= 6
+#define XNARCH_HAVE_US_ATOMIC_CMPXCHG
+
+static __inline__ void atomic_set(atomic_t *v, int i)
+{
+	unsigned long tmp;
+
+	__asm__ __volatile__("@ atomic_set\n"
+"1:	ldrex	%0, [%1]\n"
+"	strex	%0, %2, [%1]\n"
+"	teq	%0, #0\n"
+"	bne	1b"
+	: "=&r" (tmp)
+	: "r" (&v->counter), "r" (i)
+	: "cc");
+}
+
+static __inline__ int atomic_cmpxchg(atomic_t *ptr, int old, int new)
+{
+	unsigned long oldval, res;
+
+	do {
+		__asm__ __volatile__("@ atomic_cmpxchg\n"
+		"ldrex	%1, [%2]\n"
+		"mov	%0, #0\n"
+		"teq	%1, %3\n"
+		"strexeq %0, %4, [%2]\n"
+		    : "=&r" (res), "=&r" (oldval)
+		    : "r" (&ptr->counter), "Ir" (old), "r" (new)
+		    : "cc");
+	} while (res);
+
+	return oldval;
+}
+
 static __inline__ int atomic_add_return(int i, atomic_counter_t *v)
 {
     unsigned long tmp;
@@ -194,7 +239,7 @@ static __inline__ void atomic_clear_mask
     : "r" (addr), "Ir" (mask)
     : "cc");
 }
-#else /* ARM_ARCH_6 */
+#elif CONFIG_SMP
 static __inline__ int atomic_add_return(int i, atomic_counter_t *v)
 {
     int ret;
@@ -224,7 +269,122 @@ static inline void atomic_clear_mask(uns
     XENOMAI_SYSCALL3(__xn_sys_arch,
                      XENOMAI_SYSARCH_ATOMIC_CLEAR_MASK, mask, addr);
 }
-#endif /* ARM_ARCH_6 */
+#else /* ARM_ARCH <= 5 && !CONFIG_SMP */
+#define XNARCH_HAVE_US_ATOMIC_CMPXCHG
+
+static __inline__ void atomic_set(atomic_counter_t *ptr, int val)
+{
+	ptr->counter = val;
+}
+
+static __inline__ int atomic_cmpxchg(atomic_counter_t *ptr, int old, int new)
+{
+        register int asm_old asm("r0") = old;
+        register int asm_new asm("r1") = new;
+        register int *asm_ptr asm("r2") = (int *) &ptr->counter;
+        register int asm_lr asm("lr");
+	register int asm_tmp asm("r3");
+
+	do {
+		asm volatile ( \
+			"mov %1, #0xffff0fff\n\t"	\
+			"mov lr, pc\n\t"		 \
+			"add pc, %1, #(0xffff0fc0 - 0xffff0fff)\n\t"	\
+			: "+r"(asm_old), "=&r"(asm_tmp), "=r"(asm_lr)	\
+			: "r"(asm_new), "r"(asm_ptr) \
+			: "ip", "cc", "memory");
+		if (likely(!asm_old))
+			return old;
+	} while ((asm_old = *asm_ptr) == old);
+        return asm_old;
+}
+
+static __inline__ int atomic_add_return(int i, atomic_counter_t *v)
+{
+	register int asm_old asm("r0");
+	register int asm_new asm("r1");
+	register int *asm_ptr asm("r2") = (int *) &v->counter;
+        register int asm_lr asm("lr");
+	register int asm_tmp asm("r3");
+
+	asm volatile ( \
+		"1: @ atomic_add\n\t" \
+		"ldr	%0, [%4]\n\t" \
+		"mov	%1, #0xffff0fff\n\t" \
+		"add	lr, pc, #4\n\t" \
+		"add	%3, %0, %5\n\t"\
+		"add	pc, %1, #(0xffff0fc0 - 0xffff0fff)\n\t" \
+		"bcc	1b" \
+		: "=&r" (asm_old), "=&r"(asm_tmp), "=r"(asm_lr), "=r"(asm_new) \
+		: "r" (asm_ptr), "rIL"(i) \
+		: "ip", "cc", "memory");
+	return asm_new;
+}
+
+static __inline__ int atomic_sub_return(int i, atomic_counter_t *v)
+{
+	register int asm_old asm("r0");
+	register int asm_new asm("r1");
+	register int *asm_ptr asm("r2") = (int *) &v->counter;
+        register int asm_lr asm("lr");
+	register int asm_tmp asm("r3");
+
+	asm volatile ( \
+		"1: @ atomic_sub\n\t" \
+		"ldr	%0, [%4]\n\t" \
+		"mov	%1, #0xffff0fff\n\t" \
+		"add	lr, pc, #4\n\t" \
+		"sub	%3, %0, %5\n\t"\
+		"add	pc, %1, #(0xffff0fc0 - 0xffff0fff)\n\t" \
+		"bcc	1b" \
+		: "=&r" (asm_old), "=&r"(asm_tmp), "=r"(asm_lr), "=r"(asm_new) \
+		: "r" (asm_ptr), "rIL"(i) \
+		: "ip", "cc", "memory");
+	return asm_new;
+}
+
+static __inline__ void atomic_set_mask(long mask, atomic_counter_t *v)
+{
+	register int asm_old asm("r0");
+	register int asm_new asm("r1");
+	register int *asm_ptr asm("r2") = (int *) &v->counter;
+        register int asm_lr asm("lr");
+	register int asm_tmp asm("r3");
+
+	asm volatile ( \
+		"1: @ atomic_set_mask\n\t" \
+		"ldr	%0, [%4]\n\t" \
+		"mov	%1, #0xffff0fff\n\t" \
+		"add	lr, pc, #4\n\t" \
+		"orr	%3, %0, %5\n\t"\
+		"add	pc, %1, #(0xffff0fc0 - 0xffff0fff)\n\t" \
+		"bcc	1b" \
+		: "=&r" (asm_old), "=&r"(asm_tmp), "=r"(asm_lr), "=r"(asm_new) \
+		: "r" (asm_ptr), "rIL"(mask) \
+		: "ip", "cc", "memory");
+}
+
+static __inline__ void atomic_clear_mask(long mask, atomic_counter_t *v)
+{
+	register int asm_old asm("r0");
+	register int asm_new asm("r1");
+	register int *asm_ptr asm("r2") = (int *) &v->counter;
+        register int asm_lr asm("lr");
+	register int asm_tmp asm("r3");
+
+	asm volatile ( \
+		"1: @ atomic_clear_mask\n\t" \
+		"ldr	%0, [%4]\n\t" \
+		"mov	%1, #0xffff0fff\n\t" \
+		"add	lr, pc, #4\n\t" \
+		"bic	%3, %0, %5\n\t" \
+		"add	pc, %1, #(0xffff0fc0 - 0xffff0fff)\n\t" \
+		"bcc	1b" \
+		: "=&r" (asm_old), "=&r"(asm_tmp), "=r"(asm_lr), "=r"(asm_new) \
+		: "r" (asm_ptr), "rIL"(mask) \
+		: "ip", "cc", "memory");
+}
+#endif /* ARM_ARCH <= 5 && !CONFIG_SMP */
 
 #define xnarch_memory_barrier()                 __asm__ __volatile__("": : :"memory")
 
@@ -241,6 +401,9 @@ static inline void atomic_clear_mask(uns
 
 typedef unsigned long atomic_flags_t;
 
+/* Add support for atomic_long_t and atomic_ptr_t */
+#include <asm-generic/xenomai/atomic.h>
+
 #endif /* !_XENO_ASM_ARM_ATOMIC_H */
 
 // vim: ts=4 et sw=4 sts=4

-- 


					    Gilles.

[Xenomai-core] [5/9] Define new syscalls for the posix skin

[Gilles Chanteperdrix]

This patch defines the new syscalls get_heap_addr (more on this syscall later),
and get_thread_cb. The get_thread_cb syscall is used in user-space to associate
the thread xnthread_t pointer with each user-space xenomai thread, using POSIX
TSD.

This is needed for the mutex implementation to allow setting the mutex owner.

However, two things: 
- I do not know if you should like that much passing kernel-space pointers to
  user-space;
- this is implemented as a posix skin service, but should probably be defined as
  a core syscall, and called in all skin implementations, so as to allow threads
  from other skins to user Xenomai posix skin services.

---
 include/posix/syscall.h  |    3 +++
 src/skins/posix/thread.c |   11 +++++++++++
 2 files changed, 14 insertions(+)

Index: include/posix/syscall.h
===================================================================
--- include/posix/syscall.h	(revision 3718)
+++ include/posix/syscall.h	(working copy)
@@ -46,6 +46,7 @@
 #define __pse51_mutex_lock            20
 #define __pse51_mutex_timedlock       21
 #define __pse51_mutex_trylock         22
+#define __pse51_check_init            __pse51_mutex_trylock
 #define __pse51_mutex_unlock          23
 #define __pse51_cond_init             24
 #define __pse51_cond_destroy          25
@@ -101,6 +102,8 @@
 #define __pse51_thread_getschedparam  75
 #define __pse51_thread_kill           76
 #define __pse51_select                77
+#define __pse51_get_heap_addr         78
+#define __pse51_get_thread_cb	      79
 
 #ifdef __KERNEL__
 
Index: src/skins/posix/thread.c
===================================================================
--- src/skins/posix/thread.c	(revision 3718)
+++ src/skins/posix/thread.c	(working copy)
@@ -26,6 +26,7 @@
 #include <posix/syscall.h>
 
 extern int __pse51_muxid;
+extern pthread_key_t pse51_cur_key;
 
 static pthread_attr_t default_attr;
 static int linuxthreads;
@@ -41,6 +42,13 @@ static void __pthread_sigharden_handler(
 	XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_XENO_DOMAIN);
 }
 
+static void set_kthread_cb(void)
+{
+	void *kthread_cb;
+	XENOMAI_SKINCALL1(__pse51_muxid, __pse51_get_thread_cb, &kthread_cb);
+	pthread_setspecific(pse51_cur_key, kthread_cb);
+}
+
 int __wrap_pthread_setschedparam(pthread_t thread,
 				 int policy, const struct sched_param *param)
 {
@@ -56,6 +64,7 @@ int __wrap_pthread_setschedparam(pthread
 
 	if (!err && promoted) {
 		old_sigharden_handler = signal(SIGHARDEN, &__pthread_sigharden_handler);
+		set_kthread_cb();
 		if (policy != SCHED_OTHER)
 			XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_XENO_DOMAIN);
 	}
@@ -132,6 +141,8 @@ static void *__pthread_trampoline(void *
 	start = iargs->start;
 	cookie = iargs->arg;
 
+	set_kthread_cb();
+
 	__real_sem_post(&iargs->sync);
 
 	if (!err) {


-- 


					    Gilles.

[Xenomai-core] [6/9] Rework posix skin shared heaps support, add per-process shared heap.

[Gilles Chanteperdrix]

This patch removes much of the #ifdefery that was in ksrc/skins/posix/shm.c. It
also initializes a global shared heap (for process-shared objects), as well as a
per-process shared heap. These two heaps are mapped in the posix skin library
initialization, using the new get_heap_addr syscall to get their addresses.

Of course, at this chance, we use the XNARCH_SHARED_HEAP_FLAGS macro, as a flag
passed to xnheap_init_mapped.

Several questions here:
- the heaps are created with a size of three pages, hardcoded, maybe we should
  make this size somehow configurable (Kconfig, /proc, whatever). My idea was
  that each mutex will use about 4 bytes in a heap, so 12 Kbytes is plenty.
- if we do not want to waste memory, like the xnthread_t * TSD, I wonder if
  these two heaps should not be allocated by the core event callback,
  unfortunately, this would mean that we would have to associate a per-process
  data with the core skin, which we avoided until now.

---
 ksrc/skins/posix/internal.h |   10 ++++++
 ksrc/skins/posix/module.c   |   41 ++++++++++++++++++++++++++
 ksrc/skins/posix/shm.c      |   44 ++++------------------------
 src/skins/posix/init.c      |   68 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 126 insertions(+), 37 deletions(-)

Index: ksrc/skins/posix/internal.h
===================================================================
--- ksrc/skins/posix/internal.h	(revision 3718)
+++ ksrc/skins/posix/internal.h	(working copy)
@@ -71,6 +71,7 @@ typedef struct {
 	xnqueue_t semq;
 	xnqueue_t threadq;
 	xnqueue_t timerq;
+	xnheap_t shm_heap;
 } pse51_kqueues_t;
 
 #ifdef CONFIG_XENO_OPT_PERVASIVE
@@ -202,4 +203,13 @@ static inline int clock_flag(int flag, c
 int pse51_mq_select_bind(mqd_t fd, struct xnselector *selector,
 			 unsigned type, unsigned index);
 
+#ifdef CONFIG_XENO_OPT_PERVASIVE
+#define pse51_shm_heap_init(h, l, f) xnheap_init_mapped(h, l, f)
+#define pse51_shm_heap_destroy(h) xnheap_destroy_mapped(h)
+#else /* !CONFIG_XENO_OPT_PERVASIVE */
+int pse51_shm_heap_init(xnheap_t *heap, unsigned len, int flags);
+
+void pse51_shm_heap_destroy(xnheap_t *heap);
+#endif /* !CONFIG_XENO_OPT_PERVASIVE */
+
 #endif /* !_POSIX_INTERNAL_H */
Index: ksrc/skins/posix/module.c
===================================================================
--- ksrc/skins/posix/module.c	(revision 3718)
+++ ksrc/skins/posix/module.c	(working copy)
@@ -78,6 +78,36 @@ MODULE_PARM_DESC(time_slice, "Default ti
 
 xntbase_t *pse51_tbase;
 
+#ifndef CONFIG_XENO_OPT_PERVASIVE
+static void pse51_free_heap_extent(xnheap_t *heap,
+				   void *extent, u_long size, void *cookie)
+{
+	xnarch_free_host_mem(extent, size);
+}
+
+int pse51_shm_heap_init(xnheap_t *heap, unsigned len, int flags)
+{
+	void *heapaddr = xnarch_alloc_host_mem(len);
+	int err;
+
+	if (heapaddr) {
+		err = xnheap_init(&shm->heapbase,
+				  heapaddr, len, XNCORE_PAGE_SIZE);
+		if (err)
+			xnarch_free_host_mem(heapaddr, len);
+
+		return err;
+	}
+
+	return -ENOMEM;
+}
+
+void pse51_shm_heap_destroy(xnheap_t *heap)
+{
+	xnheap_destroy(heap, &pse51_free_heap_extent, NULL);
+}
+#endif /* !CONFIG_XENO_OPT_PERVASIVE */
+
 static void pse51_shutdown(int xtype)
 {
 	pse51_thread_pkg_cleanup();
@@ -98,6 +128,7 @@ static void pse51_shutdown(int xtype)
 #ifdef CONFIG_XENO_OPT_PERVASIVE
 	pse51_syscall_cleanup();
 #endif /* CONFIG_XENO_OPT_PERVASIVE */
+	pse51_shm_heap_destroy(&pse51_global_kqueues.shm_heap);
 #ifdef __KERNEL__
 	pse51_apc_pkg_cleanup();
 #endif /* __KERNEL__ */
@@ -127,11 +158,21 @@ int SKIN_INIT(posix)
 		goto fail_free_tbase;
 #endif /* __KERNEL__ */
 
+	err = pse51_shm_heap_init(&pse51_global_kqueues.shm_heap, 3 * PAGE_SIZE,
+				  (XNARCH_SHARED_HEAP_FLAGS ?: GFP_USER));
+	if (err)
+		goto fail_cleanup_apc;
+	
 #ifdef CONFIG_XENO_OPT_PERVASIVE
 	err = pse51_syscall_init();
 #endif /* CONFIG_XENO_OPT_PERVASIVE */
 	if (err != 0) {
+		pse51_shm_heap_destroy(&pse51_global_kqueues.shm_heap);
+	  fail_cleanup_apc:
+#ifdef __KERNEL__
+		pse51_apc_pkg_cleanup();
 	  fail_free_tbase:
+#endif /* __KERNEL__ */
 		xntbase_free(pse51_tbase);
 	fail_shutdown_pod:
 		xnpod_shutdown(err);
Index: ksrc/skins/posix/shm.c
===================================================================
--- ksrc/skins/posix/shm.c	(revision 3718)
+++ ksrc/skins/posix/shm.c	(working copy)
@@ -90,14 +90,6 @@ static void pse51_shm_init(pse51_shm_t *
 	appendq(&pse51_shmq, &shm->link);
 }
 
-#ifndef CONFIG_XENO_OPT_PERVASIVE
-static void pse51_free_heap_extent(xnheap_t *heap,
-				   void *extent, u_long size, void *cookie)
-{
-	xnarch_free_host_mem(extent, size);
-}
-#endif /* !CONFIG_XENO_OPT_PERVASIVE */
-
 /* Must be called nklock locked, irq off. */
 static void pse51_shm_destroy(pse51_shm_t * shm, int force)
 {
@@ -111,11 +103,7 @@ static void pse51_shm_destroy(pse51_shm_
 	if (shm->addr) {
 		xnheap_free(&shm->heapbase, shm->addr);
 
-#ifdef CONFIG_XENO_OPT_PERVASIVE
-		xnheap_destroy_mapped(&shm->heapbase);
-#else /* !CONFIG_XENO_OPT_PERVASIVE. */
-		xnheap_destroy(&shm->heapbase, &pse51_free_heap_extent, NULL);
-#endif /* !CONFIG_XENO_OPT_PERVASIVE. */
+		pse51_shm_heap_destroy(&shm->heapbase);
 
 		shm->addr = NULL;
 		shm->size = 0;
@@ -534,37 +522,19 @@ int ftruncate(int fd, off_t len)
 			memcpy(addr, shm->addr, size);
 
 			xnheap_free(&shm->heapbase, shm->addr);
-#ifdef CONFIG_XENO_OPT_PERVASIVE
-			xnheap_destroy_mapped(&shm->heapbase);
-#else /* !CONFIG_XENO_OPT_PERVASIVE. */
-			xnheap_destroy(&shm->heapbase, &pse51_free_heap_extent,
-				       NULL);
-#endif /* !CONFIG_XENO_OPT_PERVASIVE. */
+			pse51_shm_heap_destroy(&shm->heapbase);
 
 			shm->addr = NULL;
 			shm->size = 0;
 		}
 
 		if (len) {
-#ifdef CONFIG_XENO_OPT_PERVASIVE
-			int flags = len <= 128 * 1024 ? GFP_USER : 0;
-			err = -xnheap_init_mapped(&shm->heapbase, len, flags);
-#else /* !CONFIG_XENO_OPT_PERVASIVE. */
-			{
-				void *heapaddr = xnarch_alloc_host_mem(len);
-
-				if (heapaddr)
-					err =
-					    -xnheap_init(&shm->heapbase,
-							 heapaddr, len,
-							 XNCORE_PAGE_SIZE);
-				else
-					err = ENOMEM;
+			int flags = (XNARCH_SHARED_HEAP_FLAGS ?:
+				     len <= 128 * 1024 ? GFP_USER : 0);
 
-				if (err)
-					goto err_up;
-			}
-#endif /* !CONFIG_XENO_OPT_PERVASIVE. */
+			err = -pse51_shm_heap_init(&shm->heapbase, len, flags);
+			if (err)
+				goto err_up;
 
 			shm->size = xnheap_max_contiguous(&shm->heapbase);
 			shm->addr = xnheap_alloc(&shm->heapbase, shm->size);
Index: src/skins/posix/init.c
===================================================================
--- src/skins/posix/init.c	(revision 3718)
+++ src/skins/posix/init.c	(working copy)
@@ -33,10 +33,49 @@ int __pse51_muxid = -1;
 int __rtdm_muxid = -1;
 int __rtdm_fd_start = INT_MAX;
 static int fork_handler_registered;
+pthread_key_t pse51_cur_key;
+unsigned long pse51_shared_map[2] = {0, 0};
 
 int __wrap_pthread_setschedparam(pthread_t, int, const struct sched_param *);
 void pse51_clock_init(int);
 
+static void *map_shared_heap(unsigned shared)
+{
+	void *heap_addr;
+	int fd, err;
+
+	fd = __real_open("/dev/rtheap", O_RDWR, 0);
+	if (fd < 0) {
+		fprintf(stderr, "open: %m\n");
+		return MAP_FAILED;
+	}
+
+	err = XENOMAI_SKINCALL2(__pse51_muxid,
+				__pse51_get_heap_addr, &heap_addr, shared);
+	if (err < 0) {
+		fprintf(stderr, "get_heap_addr: %m\n");
+		return MAP_FAILED;
+	}
+
+	err = __real_ioctl(fd, 0, heap_addr);
+	if (err < 0) {
+		fprintf(stderr, "ioctl: %m\n");
+		return MAP_FAILED;
+	}
+
+	heap_addr = __real_mmap(NULL, 3 * getpagesize(),
+				PROT_READ | PROT_WRITE,
+				MAP_SHARED, fd, 0);
+	close(fd);
+
+	return heap_addr;
+}
+
+static void unmap_shared_heap(unsigned long heap_addr)
+{
+	__real_munmap((void *) heap_addr, 3 * getpagesize());
+}
+
 static __attribute__ ((constructor))
 void __init_posix_interface(void)
 {
@@ -83,7 +122,35 @@ void __init_posix_interface(void)
 	}
 #endif /* !CONFIG_XENO_POSIX_AUTO_MLOCKALL */
 
+	if (pse51_shared_map[0])
+		/* We have forked, so, let us recreate the private mapping */
+		unmap_shared_heap(pse51_shared_map[0]);
+	pse51_shared_map[0] = (unsigned long) map_shared_heap(0);
+	if (pse51_shared_map[0] == (unsigned long) MAP_FAILED) {
+		perror("Xenomai Posix skin init: mmap(local heap)");
+		exit(EXIT_FAILURE);
+	}
+
+	fprintf(stderr, "sizeof(pthread_mutex_t): %d,"
+		" sizeof(shadow_mutex): %d\n",
+		sizeof(pthread_mutex_t), sizeof(struct __shadow_mutex));
+	if (sizeof(struct __shadow_mutex) > sizeof(pthread_mutex_t))
+		exit(EXIT_FAILURE);
+	
 	if (!fork_handler_registered) {
+		err = pthread_key_create(&pse51_cur_key, NULL);
+		if (err) {
+			fprintf(stderr, "Xenomai Posix skin init:"
+				" pthread_key_create: %s\n", strerror(err));
+			exit(EXIT_FAILURE);
+		}
+
+		pse51_shared_map[1] = (unsigned long) map_shared_heap(1);
+		if (pse51_shared_map[1] == (unsigned long) MAP_FAILED) {
+			perror("Xenomai Posix skin init: mmap(global heap)");
+			exit(EXIT_FAILURE);
+		}
+	
 		err = pthread_atfork(NULL, NULL, &__init_posix_interface);
 		if (err) {
 			fprintf(stderr, "Xenomai Posix skin init: "
@@ -93,3 +160,4 @@ void __init_posix_interface(void)
 		fork_handler_registered = 1;
 	}
 }
+


-- 


					    Gilles.

[Xenomai-core] [7/9] Poor man's object control block read-write lock

[Gilles Chanteperdrix]

This patch adds the file ksrc/skins/posix/cb_lock.h, a header used in kernel and
user-space to protect access to mutex control blocks. It implements a kind of
read-write lock without rescheduling, a failure meaning a programming error.

pthread_mutex_lock and pthread_mutex_unlock are considered readers, several of
them are allowed to enter the lock section, they will be serialized by cmpxchg
in user-space or the nklock in the syscalls anyway, pthread_mutex_init and
pthread_mutex_destroy, however, are considered writers, only one of them may
enter the lock section, and trying to call them at the same time as a call to
pthread_mutex_lock or pthread_mutex_unlock is a programming error which is
detected and reported.

These locks boil down to locking the nklock in kernel-space when atomic_cmpxchg
is not available in user-space. But this will be removed when all architectures
define XNARCH_HAVE_US_ATOMIC_CMPXCHG.

I intend to generalize the use of this mechanism to all posix skin objects,
because they allow shorter nklock sections in kernel space: access to the object
members are protected by this read-write lock mechanism, and no longer by the
nklock with interrupts off. It also solves an issue that was opened when getting
the posix skin copy_from/to_user calls out of nklock sections.

---
 cb_lock.h |   84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)

Index: ksrc/skins/posix/cb_lock.h
===================================================================
--- ksrc/skins/posix/cb_lock.h	(revision 0)
+++ ksrc/skins/posix/cb_lock.h	(revision 0)
@@ -0,0 +1,84 @@
+#ifndef CB_LOCK_H
+#define CB_LOCK_H
+
+#include <asm/xenomai/atomic.h>
+
+#ifndef __KERNEL__
+typedef void xnthread_t;
+#endif /* __KERNEL__ */
+
+#ifdef XNARCH_HAVE_US_ATOMIC_CMPXCHG
+
+#define test_claimed(owner) ((long) (owner) & 1)
+#define clear_claimed(owner) ((xnthread_t *) ((long) (owner) & ~1))
+#define set_claimed(owner, bit) \
+        ((xnthread_t *) ((long) clear_claimed(owner) | !!(bit)))
+	
+static  __inline__ int __cb_try_read_lock(atomic_t *lock)
+{
+	unsigned val = atomic_read(lock);
+	while (likely(val != -1)) {
+		unsigned old = atomic_cmpxchg(lock, val, val + 1);
+		if (likely(old == val))
+			return 0;
+		val = old;
+	}
+	return -EBUSY;
+}
+
+static __inline__ void __cb_read_unlock(atomic_t *lock)
+{
+	unsigned old, val = atomic_read(lock);
+	while (likely(val != -1)) {
+		old = atomic_cmpxchg(lock, val, val - 1);
+		if (likely(old == val))
+			return;
+		val = old;
+	}
+}
+
+static __inline__ int __cb_try_write_lock(atomic_t *lock)
+{
+	unsigned old = atomic_cmpxchg(lock, 0, -1);
+	if (unlikely(old))
+		return -EBUSY;
+	return 0;
+}
+
+static __inline__ void __cb_force_write_lock(atomic_t *lock)
+{
+	atomic_set(lock, -1);
+}
+
+static __inline__ void __cb_write_unlock(atomic_t *lock)
+{
+	atomic_set(lock, 0);
+}
+#define DECLARE_CB_LOCK_FLAGS(name) struct { } name __attribute__((unused))
+#define cb_try_read_lock(lock, flags) __cb_try_read_lock(lock)
+#define cb_read_unlock(lock, flags) __cb_read_unlock(lock)
+#define cb_try_write_lock(lock, flags) __cb_try_write_lock(lock)
+#define cb_force_write_lock(lock, flags) __cb_force_write_lock(lock)
+#define cb_write_unlock(lock, flags) __cb_write_unlock(lock)
+#else /* !XNARCH_HAVE_US_ATOMIC_CMPXCHG */
+#ifdef __KERNEL__
+#define DECLARE_CB_LOCK_FLAGS(name) spl_t name
+#define cb_try_read_lock(lock, flags) \
+	({ xnlock_get_irqsave(&nklock, flags); 0 })
+#define cb_read_unlock(lock, flags) xnlock_put_irqrestore(&nklock, flags)
+#define cb_try_write_lock(lock, flags)  \
+	({ xnlock_get_irqsave(&nklock, flags); 0 })
+#define cb_force_write_lock(lock, flags)  \
+	({ xnlock_get_irqsave(&nklock, flags); 0 })
+#define cb_write_unlock(lock, flags) xnlock_put_irqrestore(&nklock, flags)
+#else /* !__KERNEL__ */
+#define DECLARE_CB_LOCK_FLAGS(name)
+#define cb_try_read_lock(lock, flags) (0)
+#define cb_read_unlock(lock, flags) do { } while (0)
+#define cb_try_write_lock(lock, flags) (0)
+#define cb_force_write_lock(lock, flags) do { } while (0)
+#define cb_write_unlock(lock, flags) do { } while (0)
+#endif /* !__KERNEL__ */
+#endif /* !XNARCH_HAVE_US_ATOMIC_CMPXCHG */
+
+#endif /* CB_LOCK_H */


-- 


					    Gilles.

[Xenomai-core] [8/9] Re-implementation of mutexes, kernel-space support.

[Gilles Chanteperdrix]

The new implementation of mutexes consists of an atomic_cmpxchg on the mutex
owner, using the classical test and set raises problems for doing this and
setting the mutex owner atomically, so we do it all at once.

The room for storing the mutex owner is allocated in a shared heap, so that it
can be shared between kernel and user-space. I wonder if we need this at all. If
it is possible to call atomic_cmpxchg in kernel-space specifying a user-space
address, a bit like what futex_atomic_cmpxchg_inatomic does on platforms where
it is implemented, then we do not need it. But since this function is not
implemented for any of the architectures supported by Xenomai, I preferred to
stay with the heap implementation.

Further, the owner bit 0 (unused in an xnthread_t * pointer, since it is aligned
at least on a sizeof(int)), is used to mean that the mutex is claimed, to have
the atomic_cmpxchg used by the user-space mutex_unlock fail and cause the call
to the syscall, which will trigger xnsynch_wakeup_one_sleeper.

The implementation of pthread_cond_wait is also slightly modified, the mutex
condvars counter, used to forbid a call to pthread_mutex_destroy while
pthread_cond_wait is suspended, disappeared in favour of a locking by
pthread_cond_wait of the mutex control-block read-write lock.

The function used by pthread_cond_wait to unlock a mutex, possibly saving its
recursion count also needed to be rewritten.

---
 include/posix/pthread.h    |   56 ++++---
 ksrc/skins/posix/cond.c    |   41 +++--
 ksrc/skins/posix/mutex.c   |  296 ++++++++++++++++++++++++----------------
 ksrc/skins/posix/mutex.h   |  123 ++++++++++++----
 ksrc/skins/posix/syscall.c |  329 +++++++++++++++++++++++++++++++++++++++++++--
 5 files changed, 651 insertions(+), 194 deletions(-)

Index: src/skins/posix/mutex.c
===================================================================
--- src/skins/posix/mutex.c	(revision 3718)
+++ src/skins/posix/mutex.c	(working copy)
@@ -18,9 +18,14 @@
 
 #include <errno.h>
 #include <posix/syscall.h>
+#include <posix/cb_lock.h>
 #include <pthread.h>
 
+#define PSE51_MUTEX_MAGIC (0x86860303)
+
 extern int __pse51_muxid;
+extern pthread_key_t pse51_cur_key;
+extern unsigned long pse51_shared_map[2];
 
 int __wrap_pthread_mutexattr_init(pthread_mutexattr_t *attr)
 {
@@ -73,66 +78,279 @@ int __wrap_pthread_mutexattr_setpshared(
 				  __pse51_mutexattr_setpshared, attr, pshared);
 }
 
-int __wrap_pthread_mutex_init(pthread_mutex_t * mutex,
-			      const pthread_mutexattr_t * attr)
+static atomic_ptr_t *get_ownerp(struct __shadow_mutex *shadow)
+{
+	if (likely(!shadow->attr.pshared))
+		return shadow->owner;
+	
+	return (atomic_ptr_t *) (pse51_shared_map[1] + shadow->owner_offset);
+}
+
+int __wrap_pthread_mutex_init(pthread_mutex_t *mutex,
+			      const pthread_mutexattr_t *attr)
 {
 	union __xeno_mutex *_mutex = (union __xeno_mutex *)mutex;
+	struct __shadow_mutex *shadow = &_mutex->shadow_mutex;
 	int err;
 
-	err = -XENOMAI_SKINCALL2(__pse51_muxid,
-				 __pse51_mutex_init,&_mutex->shadow_mutex,attr);
+#ifdef XNARCH_HAVE_US_ATOMIC_CMPXCHG
+	if (unlikely(cb_try_read_lock(&shadow->lock, s)))
+		goto checked;
+
+	err = -XENOMAI_SKINCALL2(__pse51_muxid,__pse51_check_init,shadow,attr);
+
+	if (err) {
+		cb_read_unlock(&shadow->lock, s);
+		return err;
+	}
+
+  checked:
+	cb_force_write_lock(&shadow->lock, s);
+#endif /* XNARCH_HAVE_US_ATOMIC_CMPXCHG */
+
+	err = -XENOMAI_SKINCALL2(__pse51_muxid,__pse51_mutex_init,shadow,attr);
+
+	if (!shadow->attr.pshared)
+		shadow->owner = (atomic_ptr_t *)
+			(pse51_shared_map[0] + shadow->owner_offset);
+	
+	cb_write_unlock(&shadow->lock, s);
+
 	return err;
 }
 
-int __wrap_pthread_mutex_destroy(pthread_mutex_t * mutex)
+int __wrap_pthread_mutex_destroy(pthread_mutex_t *mutex)
 {
 	union __xeno_mutex *_mutex = (union __xeno_mutex *)mutex;
+	struct __shadow_mutex *shadow = &_mutex->shadow_mutex;
+	int err;
 
-	return -XENOMAI_SKINCALL1(__pse51_muxid,
-				  __pse51_mutex_destroy, &_mutex->shadow_mutex);
+	if (unlikely(cb_try_write_lock(&shadow->lock, s)))
+		return EINVAL;
+
+	err = -XENOMAI_SKINCALL1(__pse51_muxid, __pse51_mutex_destroy, shadow);
+
+	cb_write_unlock(&shadow->lock, s);
+
+	return err;
 }
 
-int __wrap_pthread_mutex_lock(pthread_mutex_t * mutex)
+int __wrap_pthread_mutex_lock(pthread_mutex_t *mutex)
 {
 	union __xeno_mutex *_mutex = (union __xeno_mutex *)mutex;
-	int err;
+	struct __shadow_mutex *shadow = &_mutex->shadow_mutex;
+	xnthread_t *cur, *owner;
+	int err = 0;
+
+#ifdef XNARCH_HAVE_US_ATOMIC_CMPXCHG
+	cur = pthread_getspecific(pse51_cur_key);
+	if (!cur)
+		return EPERM;
+
+	if (unlikely(cb_try_read_lock(&shadow->lock, s)))
+		return EINVAL;
+
+	if (unlikely(shadow->magic != PSE51_MUTEX_MAGIC)) {
+		err = -EINVAL;
+		goto out;
+	}
+
+	owner = atomic_ptr_cmpxchg(get_ownerp(shadow), NULL, cur);
+	if (likely(!owner)) {
+		shadow->lockcnt = 1;
+		cb_read_unlock(&shadow->lock, s);
+		return 0;
+	}
+
+	if (clear_claimed(owner) == cur)
+		switch(shadow->attr.type) {
+		case PTHREAD_MUTEX_NORMAL:
+			break;
+
+		case PTHREAD_MUTEX_ERRORCHECK:
+			err = -EDEADLK;
+			goto out;
+
+		case PTHREAD_MUTEX_RECURSIVE:
+			if (shadow->lockcnt == UINT_MAX) {
+				err = -EAGAIN;
+				goto out;
+			}
+
+			++shadow->lockcnt;
+			goto out;
+		}
+#endif /* XNARCH_HAVE_US_ATOMIC_CMPXCHG */
 
 	do {
-		err = XENOMAI_SKINCALL1(__pse51_muxid,
-					__pse51_mutex_lock,
-					&_mutex->shadow_mutex);
+		err = XENOMAI_SKINCALL1(__pse51_muxid,__pse51_mutex_lock,shadow);
 	} while (err == -EINTR);
 
+#ifdef XNARCH_HAVE_US_ATOMIC_CMPXCHG
+  out:
+	cb_read_unlock(&shadow->lock, s);
+#endif /* XNARCH_HAVE_US_ATOMIC_CMPXCHG */
+
 	return -err;
 }
 
-int __wrap_pthread_mutex_timedlock(pthread_mutex_t * mutex,
+int __wrap_pthread_mutex_timedlock(pthread_mutex_t *mutex,
 				   const struct timespec *to)
 {
 	union __xeno_mutex *_mutex = (union __xeno_mutex *)mutex;
-	int err;
+	struct __shadow_mutex *shadow = &_mutex->shadow_mutex;
+	xnthread_t *cur, *owner;
+	int err = 0;
+
+#ifdef XNARCH_HAVE_US_ATOMIC_CMPXCHG
+	cur = pthread_getspecific(pse51_cur_key);
+	if (!cur)
+		return EPERM;
+
+	if (unlikely(cb_try_read_lock(&shadow->lock, s)))
+		return EINVAL;
+
+	if (shadow->magic != PSE51_MUTEX_MAGIC) {
+		err = -EINVAL;
+		goto out;
+	}	
+
+	owner = atomic_ptr_cmpxchg(get_ownerp(shadow), NULL, cur);
+	if (likely(!owner)) {
+		shadow->lockcnt = 1;
+		cb_read_unlock(&shadow->lock, s);
+		return 0;
+	}
+
+	if (clear_claimed(owner) == cur)
+		switch(shadow->attr.type) {
+		case PTHREAD_MUTEX_NORMAL:
+			break;
+
+		case PTHREAD_MUTEX_ERRORCHECK:
+			err = -EDEADLK;
+			goto out;
+
+		case PTHREAD_MUTEX_RECURSIVE:
+			if (shadow->lockcnt == UINT_MAX) {
+				err = -EAGAIN;
+				goto out;
+			}
+
+			++shadow->lockcnt;
+			goto out;
+		}
+#endif /* XNARCH_HAVE_US_ATOMIC_CMPXCHG */
 
 	do {
 		err = XENOMAI_SKINCALL2(__pse51_muxid,
-					__pse51_mutex_timedlock,
-					&_mutex->shadow_mutex, to);
+					__pse51_mutex_timedlock, shadow, to);
 	} while (err == -EINTR);
 
+#ifdef XNARCH_HAVE_US_ATOMIC_CMPXCHG
+  out:
+	cb_read_unlock(&shadow->lock, s);
+#endif /* XNARCH_HAVE_US_ATOMIC_CMPXCHG */
+
 	return -err;
 }
 
-int __wrap_pthread_mutex_trylock(pthread_mutex_t * mutex)
+int __wrap_pthread_mutex_trylock(pthread_mutex_t *mutex)
 {
 	union __xeno_mutex *_mutex = (union __xeno_mutex *)mutex;
+	struct __shadow_mutex *shadow = &_mutex->shadow_mutex;
+	xnthread_t *cur, *owner;
+	int err = 0;
+
+#ifdef XNARCH_HAVE_US_ATOMIC_CMPXCHG
+	cur = pthread_getspecific(pse51_cur_key);
+	if (!cur)
+		return EPERM;
+
+	if (unlikely(cb_try_read_lock(&shadow->lock, s)))
+		return EINVAL;
+
+	if (unlikely(shadow->magic != PSE51_MUTEX_MAGIC)) {
+		err = -EINVAL;
+		goto out;
+	}	
+
+	owner = atomic_ptr_cmpxchg(get_ownerp(shadow), NULL, cur);
+	if (likely(!owner)) {
+		shadow->lockcnt = 1;
+		cb_read_unlock(&shadow->lock, s);
+		return 0;
+	}
+
+	err = -EBUSY;
+	if (clear_claimed(owner) == cur
+	    && shadow->attr.type == PTHREAD_MUTEX_RECURSIVE)
+		if (shadow->lockcnt == UINT_MAX)
+			err = -EAGAIN;
+		else {
+			++shadow->lockcnt;
+			err = 0;
+		}
+
+  out:
+	cb_read_unlock(&shadow->lock, s);
+
+	return -err;
+
+#else /* !XNARCH_HAVE_US_ATOMIC_CMPXCHG */
+
+	return -XENOMAI_SKINCALL1(__pse51_muxid, __pse51_mutex_trylock, shadow);
 
-	return -XENOMAI_SKINCALL1(__pse51_muxid,
-				  __pse51_mutex_trylock, &_mutex->shadow_mutex);
+#endif /* !XNARCH_HAVE_US_ATOMIC_CMPXCHG */
 }
 
-int __wrap_pthread_mutex_unlock(pthread_mutex_t * mutex)
+int __wrap_pthread_mutex_unlock(pthread_mutex_t *mutex)
 {
 	union __xeno_mutex *_mutex = (union __xeno_mutex *)mutex;
+	struct __shadow_mutex *shadow = &_mutex->shadow_mutex;
 
-	return -XENOMAI_SKINCALL1(__pse51_muxid,
-				  __pse51_mutex_unlock, &_mutex->shadow_mutex);
+#ifdef XNARCH_HAVE_US_ATOMIC_CMPXCHG
+	atomic_ptr_t *ownerp;
+	xnthread_t *cur;
+	int err = 0;
+
+	cur = pthread_getspecific(pse51_cur_key);
+	if (!cur)
+		return EPERM;
+
+	if (unlikely(cb_try_read_lock(&shadow->lock, s)))
+		return EINVAL;
+
+	if (unlikely(shadow->magic != PSE51_MUTEX_MAGIC)) {
+		err = -EINVAL;
+		goto out_err;
+	}
+
+	ownerp = get_ownerp(shadow);
+	if (unlikely(clear_claimed(atomic_ptr_read(ownerp)) != cur)) {
+		err = -EPERM;
+		goto out_err;
+	}
+
+	err = 0;
+	if (shadow->lockcnt > 1) {
+		--shadow->lockcnt;
+		goto out;
+	}
+
+	if (likely(atomic_ptr_cmpxchg(ownerp, cur, NULL) == cur)) {
+	  out:
+		cb_read_unlock(&shadow->lock, s);
+		return 0;
+	}
+#endif /* XNARCH_HAVE_US_ATOMIC_CMPXCHG */
+
+	err = XENOMAI_SKINCALL1(__pse51_muxid, __pse51_mutex_unlock, shadow);
+
+#ifdef XNARCH_HAVE_US_ATOMIC_CMPXCHG
+  out_err:
+	cb_read_unlock(&shadow->lock, s);
+#endif /* XNARCH_HAVE_US_ATOMIC_CMPXCHG */
+
+	return -err;
 }
Index: src/skins/posix/cond.c
===================================================================
--- src/skins/posix/cond.c	(revision 3718)
+++ src/skins/posix/cond.c	(working copy)
@@ -19,6 +19,7 @@
 #include <errno.h>
 #include <posix/syscall.h>
 #include <pthread.h>
+#include <posix/cb_lock.h>
 
 extern int __pse51_muxid;
 
@@ -95,7 +96,7 @@ static void __pthread_cond_cleanup(void 
 			  c->count);
 }
 
-int __wrap_pthread_cond_wait(pthread_cond_t * cond, pthread_mutex_t * mutex)
+int __wrap_pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *mutex)
 {
 	struct pse51_cond_cleanup_t c = {
 		.cond = (union __xeno_cond *)cond,
@@ -103,6 +104,9 @@ int __wrap_pthread_cond_wait(pthread_con
 	};
 	int err, oldtype;
 
+	if (cb_try_read_lock(&c.mutex->shadow_mutex.lock, s))
+		return EINVAL;
+
 	pthread_cleanup_push(&__pthread_cond_cleanup, &c);
 
 	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, &oldtype);
@@ -118,11 +122,15 @@ int __wrap_pthread_cond_wait(pthread_con
 
 	pthread_cleanup_pop(0);
 
-	if (err)
+	if (err) {
+		cb_read_unlock(&c.mutex->shadow_mutex.lock, s);
 		return err;
+	}
 
 	__pthread_cond_cleanup(&c);
 
+	cb_read_unlock(&c.mutex->shadow_mutex.lock, s);
+
 	pthread_testcancel();
 
 	return 0;
@@ -138,6 +146,9 @@ int __wrap_pthread_cond_timedwait(pthrea
 	};
 	int err, oldtype;
 
+	if (cb_try_read_lock(&c.mutex->shadow_mutex.lock, s))
+		return EINVAL;
+
 	pthread_cleanup_push(&__pthread_cond_cleanup, &c);
 
 	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, &oldtype);
@@ -153,11 +164,15 @@ int __wrap_pthread_cond_timedwait(pthrea
 
 	pthread_cleanup_pop(0);
 
-	if (err && err != ETIMEDOUT)
+	if (err && err != ETIMEDOUT) {
+		cb_read_unlock(&c.mutex->shadow_mutex.lock, s);		
 		return err;
+	}
 
 	__pthread_cond_cleanup(&c);
 
+	cb_read_unlock(&c.mutex->shadow_mutex.lock, s);
+
 	pthread_testcancel();
 
 	return err;
Index: src/skins/posix/Makefile.am
===================================================================
--- src/skins/posix/Makefile.am	(revision 3718)
+++ src/skins/posix/Makefile.am	(working copy)
@@ -2,6 +2,8 @@ includedir = $(prefix)/include/posix
 
 lib_LTLIBRARIES = libpthread_rt.la
 
+CPPFLAGS+=-I$(top_srcdir)/ksrc/skins
+
 libpthread_rt_la_LDFLAGS = -version-info 1:0:0 -lpthread
 
 libpthread_rt_la_SOURCES = \


-- 


					    Gilles.

[Xenomai-core] [9/9] Re-implementation of mutex, user-space support.

[Gilles Chanteperdrix]

The new implementation. In user-space, without syscall in the common case when a
mutex is free when locking it, and was not claimed when unlocking it.

Note that this change entails a change in behaviour of the mutexes: before this
change locking a mutex caused the calling thread to switch to primary mode. Now,
the thread locking a mutex without syscall remains in the same mode.

---
 Makefile.am |    2
 cond.c      |   21 ++++
 mutex.c     |  262 ++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 3 files changed, 260 insertions(+), 25 deletions(-)

Index: src/skins/posix/mutex.c
===================================================================
--- src/skins/posix/mutex.c	(revision 3718)
+++ src/skins/posix/mutex.c	(working copy)
@@ -18,9 +18,14 @@
 
 #include <errno.h>
 #include <posix/syscall.h>
+#include <posix/cb_lock.h>
 #include <pthread.h>
 
+#define PSE51_MUTEX_MAGIC (0x86860303)
+
 extern int __pse51_muxid;
+extern pthread_key_t pse51_cur_key;
+extern unsigned long pse51_shared_map[2];
 
 int __wrap_pthread_mutexattr_init(pthread_mutexattr_t *attr)
 {
@@ -73,66 +78,279 @@ int __wrap_pthread_mutexattr_setpshared(
 				  __pse51_mutexattr_setpshared, attr, pshared);
 }
 
-int __wrap_pthread_mutex_init(pthread_mutex_t * mutex,
-			      const pthread_mutexattr_t * attr)
+static atomic_ptr_t *get_ownerp(struct __shadow_mutex *shadow)
+{
+	if (likely(!shadow->attr.pshared))
+		return shadow->owner;
+	
+	return (atomic_ptr_t *) (pse51_shared_map[1] + shadow->owner_offset);
+}
+
+int __wrap_pthread_mutex_init(pthread_mutex_t *mutex,
+			      const pthread_mutexattr_t *attr)
 {
 	union __xeno_mutex *_mutex = (union __xeno_mutex *)mutex;
+	struct __shadow_mutex *shadow = &_mutex->shadow_mutex;
 	int err;
 
-	err = -XENOMAI_SKINCALL2(__pse51_muxid,
-				 __pse51_mutex_init,&_mutex->shadow_mutex,attr);
+#ifdef XNARCH_HAVE_US_ATOMIC_CMPXCHG
+	if (unlikely(cb_try_read_lock(&shadow->lock, s)))
+		goto checked;
+
+	err = -XENOMAI_SKINCALL2(__pse51_muxid,__pse51_check_init,shadow,attr);
+
+	if (err) {
+		cb_read_unlock(&shadow->lock, s);
+		return err;
+	}
+
+  checked:
+	cb_force_write_lock(&shadow->lock, s);
+#endif /* XNARCH_HAVE_US_ATOMIC_CMPXCHG */
+
+	err = -XENOMAI_SKINCALL2(__pse51_muxid,__pse51_mutex_init,shadow,attr);
+
+	if (!shadow->attr.pshared)
+		shadow->owner = (atomic_ptr_t *)
+			(pse51_shared_map[0] + shadow->owner_offset);
+	
+	cb_write_unlock(&shadow->lock, s);
+
 	return err;
 }
 
-int __wrap_pthread_mutex_destroy(pthread_mutex_t * mutex)
+int __wrap_pthread_mutex_destroy(pthread_mutex_t *mutex)
 {
 	union __xeno_mutex *_mutex = (union __xeno_mutex *)mutex;
+	struct __shadow_mutex *shadow = &_mutex->shadow_mutex;
+	int err;
 
-	return -XENOMAI_SKINCALL1(__pse51_muxid,
-				  __pse51_mutex_destroy, &_mutex->shadow_mutex);
+	if (unlikely(cb_try_write_lock(&shadow->lock, s)))
+		return EINVAL;
+
+	err = -XENOMAI_SKINCALL1(__pse51_muxid, __pse51_mutex_destroy, shadow);
+
+	cb_write_unlock(&shadow->lock, s);
+
+	return err;
 }
 
-int __wrap_pthread_mutex_lock(pthread_mutex_t * mutex)
+int __wrap_pthread_mutex_lock(pthread_mutex_t *mutex)
 {
 	union __xeno_mutex *_mutex = (union __xeno_mutex *)mutex;
-	int err;
+	struct __shadow_mutex *shadow = &_mutex->shadow_mutex;
+	xnthread_t *cur, *owner;
+	int err = 0;
+
+#ifdef XNARCH_HAVE_US_ATOMIC_CMPXCHG
+	cur = pthread_getspecific(pse51_cur_key);
+	if (!cur)
+		return EPERM;
+
+	if (unlikely(cb_try_read_lock(&shadow->lock, s)))
+		return EINVAL;
+
+	if (unlikely(shadow->magic != PSE51_MUTEX_MAGIC)) {
+		err = -EINVAL;
+		goto out;
+	}
+
+	owner = atomic_ptr_cmpxchg(get_ownerp(shadow), NULL, cur);
+	if (likely(!owner)) {
+		shadow->lockcnt = 1;
+		cb_read_unlock(&shadow->lock, s);
+		return 0;
+	}
+
+	if (clear_claimed(owner) == cur)
+		switch(shadow->attr.type) {
+		case PTHREAD_MUTEX_NORMAL:
+			break;
+
+		case PTHREAD_MUTEX_ERRORCHECK:
+			err = -EDEADLK;
+			goto out;
+
+		case PTHREAD_MUTEX_RECURSIVE:
+			if (shadow->lockcnt == UINT_MAX) {
+				err = -EAGAIN;
+				goto out;
+			}
+
+			++shadow->lockcnt;
+			goto out;
+		}
+#endif /* XNARCH_HAVE_US_ATOMIC_CMPXCHG */
 
 	do {
-		err = XENOMAI_SKINCALL1(__pse51_muxid,
-					__pse51_mutex_lock,
-					&_mutex->shadow_mutex);
+		err = XENOMAI_SKINCALL1(__pse51_muxid,__pse51_mutex_lock,shadow);
 	} while (err == -EINTR);
 
+#ifdef XNARCH_HAVE_US_ATOMIC_CMPXCHG
+  out:
+	cb_read_unlock(&shadow->lock, s);
+#endif /* XNARCH_HAVE_US_ATOMIC_CMPXCHG */
+
 	return -err;
 }
 
-int __wrap_pthread_mutex_timedlock(pthread_mutex_t * mutex,
+int __wrap_pthread_mutex_timedlock(pthread_mutex_t *mutex,
 				   const struct timespec *to)
 {
 	union __xeno_mutex *_mutex = (union __xeno_mutex *)mutex;
-	int err;
+	struct __shadow_mutex *shadow = &_mutex->shadow_mutex;
+	xnthread_t *cur, *owner;
+	int err = 0;
+
+#ifdef XNARCH_HAVE_US_ATOMIC_CMPXCHG
+	cur = pthread_getspecific(pse51_cur_key);
+	if (!cur)
+		return EPERM;
+
+	if (unlikely(cb_try_read_lock(&shadow->lock, s)))
+		return EINVAL;
+
+	if (shadow->magic != PSE51_MUTEX_MAGIC) {
+		err = -EINVAL;
+		goto out;
+	}	
+
+	owner = atomic_ptr_cmpxchg(get_ownerp(shadow), NULL, cur);
+	if (likely(!owner)) {
+		shadow->lockcnt = 1;
+		cb_read_unlock(&shadow->lock, s);
+		return 0;
+	}
+
+	if (clear_claimed(owner) == cur)
+		switch(shadow->attr.type) {
+		case PTHREAD_MUTEX_NORMAL:
+			break;
+
+		case PTHREAD_MUTEX_ERRORCHECK:
+			err = -EDEADLK;
+			goto out;
+
+		case PTHREAD_MUTEX_RECURSIVE:
+			if (shadow->lockcnt == UINT_MAX) {
+				err = -EAGAIN;
+				goto out;
+			}
+
+			++shadow->lockcnt;
+			goto out;
+		}
+#endif /* XNARCH_HAVE_US_ATOMIC_CMPXCHG */
 
 	do {
 		err = XENOMAI_SKINCALL2(__pse51_muxid,
-					__pse51_mutex_timedlock,
-					&_mutex->shadow_mutex, to);
+					__pse51_mutex_timedlock, shadow, to);
 	} while (err == -EINTR);
 
+#ifdef XNARCH_HAVE_US_ATOMIC_CMPXCHG
+  out:
+	cb_read_unlock(&shadow->lock, s);
+#endif /* XNARCH_HAVE_US_ATOMIC_CMPXCHG */
+
 	return -err;
 }
 
-int __wrap_pthread_mutex_trylock(pthread_mutex_t * mutex)
+int __wrap_pthread_mutex_trylock(pthread_mutex_t *mutex)
 {
 	union __xeno_mutex *_mutex = (union __xeno_mutex *)mutex;
+	struct __shadow_mutex *shadow = &_mutex->shadow_mutex;
+	xnthread_t *cur, *owner;
+	int err = 0;
+
+#ifdef XNARCH_HAVE_US_ATOMIC_CMPXCHG
+	cur = pthread_getspecific(pse51_cur_key);
+	if (!cur)
+		return EPERM;
+
+	if (unlikely(cb_try_read_lock(&shadow->lock, s)))
+		return EINVAL;
+
+	if (unlikely(shadow->magic != PSE51_MUTEX_MAGIC)) {
+		err = -EINVAL;
+		goto out;
+	}	
+
+	owner = atomic_ptr_cmpxchg(get_ownerp(shadow), NULL, cur);
+	if (likely(!owner)) {
+		shadow->lockcnt = 1;
+		cb_read_unlock(&shadow->lock, s);
+		return 0;
+	}
+
+	err = -EBUSY;
+	if (clear_claimed(owner) == cur
+	    && shadow->attr.type == PTHREAD_MUTEX_RECURSIVE)
+		if (shadow->lockcnt == UINT_MAX)
+			err = -EAGAIN;
+		else {
+			++shadow->lockcnt;
+			err = 0;
+		}
+
+  out:
+	cb_read_unlock(&shadow->lock, s);
+
+	return -err;
+
+#else /* !XNARCH_HAVE_US_ATOMIC_CMPXCHG */
+
+	return -XENOMAI_SKINCALL1(__pse51_muxid, __pse51_mutex_trylock, shadow);
 
-	return -XENOMAI_SKINCALL1(__pse51_muxid,
-				  __pse51_mutex_trylock, &_mutex->shadow_mutex);
+#endif /* !XNARCH_HAVE_US_ATOMIC_CMPXCHG */
 }
 
-int __wrap_pthread_mutex_unlock(pthread_mutex_t * mutex)
+int __wrap_pthread_mutex_unlock(pthread_mutex_t *mutex)
 {
 	union __xeno_mutex *_mutex = (union __xeno_mutex *)mutex;
+	struct __shadow_mutex *shadow = &_mutex->shadow_mutex;
 
-	return -XENOMAI_SKINCALL1(__pse51_muxid,
-				  __pse51_mutex_unlock, &_mutex->shadow_mutex);
+#ifdef XNARCH_HAVE_US_ATOMIC_CMPXCHG
+	atomic_ptr_t *ownerp;
+	xnthread_t *cur;
+	int err = 0;
+
+	cur = pthread_getspecific(pse51_cur_key);
+	if (!cur)
+		return EPERM;
+
+	if (unlikely(cb_try_read_lock(&shadow->lock, s)))
+		return EINVAL;
+
+	if (unlikely(shadow->magic != PSE51_MUTEX_MAGIC)) {
+		err = -EINVAL;
+		goto out_err;
+	}
+
+	ownerp = get_ownerp(shadow);
+	if (unlikely(clear_claimed(atomic_ptr_read(ownerp)) != cur)) {
+		err = -EPERM;
+		goto out_err;
+	}
+
+	err = 0;
+	if (shadow->lockcnt > 1) {
+		--shadow->lockcnt;
+		goto out;
+	}
+
+	if (likely(atomic_ptr_cmpxchg(ownerp, cur, NULL) == cur)) {
+	  out:
+		cb_read_unlock(&shadow->lock, s);
+		return 0;
+	}
+#endif /* XNARCH_HAVE_US_ATOMIC_CMPXCHG */
+
+	err = XENOMAI_SKINCALL1(__pse51_muxid, __pse51_mutex_unlock, shadow);
+
+#ifdef XNARCH_HAVE_US_ATOMIC_CMPXCHG
+  out_err:
+	cb_read_unlock(&shadow->lock, s);
+#endif /* XNARCH_HAVE_US_ATOMIC_CMPXCHG */
+
+	return -err;
 }
Index: src/skins/posix/cond.c
===================================================================
--- src/skins/posix/cond.c	(revision 3718)
+++ src/skins/posix/cond.c	(working copy)
@@ -19,6 +19,7 @@
 #include <errno.h>
 #include <posix/syscall.h>
 #include <pthread.h>
+#include <posix/cb_lock.h>
 
 extern int __pse51_muxid;
 
@@ -95,7 +96,7 @@ static void __pthread_cond_cleanup(void 
 			  c->count);
 }
 
-int __wrap_pthread_cond_wait(pthread_cond_t * cond, pthread_mutex_t * mutex)
+int __wrap_pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *mutex)
 {
 	struct pse51_cond_cleanup_t c = {
 		.cond = (union __xeno_cond *)cond,
@@ -103,6 +104,9 @@ int __wrap_pthread_cond_wait(pthread_con
 	};
 	int err, oldtype;
 
+	if (cb_try_read_lock(&c.mutex->shadow_mutex.lock, s))
+		return EINVAL;
+
 	pthread_cleanup_push(&__pthread_cond_cleanup, &c);
 
 	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, &oldtype);
@@ -118,11 +122,15 @@ int __wrap_pthread_cond_wait(pthread_con
 
 	pthread_cleanup_pop(0);
 
-	if (err)
+	if (err) {
+		cb_read_unlock(&c.mutex->shadow_mutex.lock, s);
 		return err;
+	}
 
 	__pthread_cond_cleanup(&c);
 
+	cb_read_unlock(&c.mutex->shadow_mutex.lock, s);
+
 	pthread_testcancel();
 
 	return 0;
@@ -138,6 +146,9 @@ int __wrap_pthread_cond_timedwait(pthrea
 	};
 	int err, oldtype;
 
+	if (cb_try_read_lock(&c.mutex->shadow_mutex.lock, s))
+		return EINVAL;
+
 	pthread_cleanup_push(&__pthread_cond_cleanup, &c);
 
 	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, &oldtype);
@@ -153,11 +164,15 @@ int __wrap_pthread_cond_timedwait(pthrea
 
 	pthread_cleanup_pop(0);
 
-	if (err && err != ETIMEDOUT)
+	if (err && err != ETIMEDOUT) {
+		cb_read_unlock(&c.mutex->shadow_mutex.lock, s);		
 		return err;
+	}
 
 	__pthread_cond_cleanup(&c);
 
+	cb_read_unlock(&c.mutex->shadow_mutex.lock, s);
+
 	pthread_testcancel();
 
 	return err;
Index: src/skins/posix/Makefile.am
===================================================================
--- src/skins/posix/Makefile.am	(revision 3718)
+++ src/skins/posix/Makefile.am	(working copy)
@@ -2,6 +2,8 @@ includedir = $(prefix)/include/posix
 
 lib_LTLIBRARIES = libpthread_rt.la
 
+CPPFLAGS+=-I$(top_srcdir)/ksrc/skins
+
 libpthread_rt_la_LDFLAGS = -version-info 1:0:0 -lpthread
 
 libpthread_rt_la_SOURCES = \


-- 


					    Gilles.

Re: [Xenomai-core] [0/9] Posix skin user-space mutexes

[Jan Kiszka]

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

Gilles Chanteperdrix wrote:
> Hi,
> 
> the patch series to come for review adds support for user-space mutexes to the
> posix skin. Since I wanted this support to be available on my AT91RM9200, the
> patch series start with patches which are mainly for the ARM architecture, to
> end with reimplementation of the kernel-space and user-space mutex services for
> the posix skin.

Cool stuff!

> 
> Some patches brought some questions when making them, questions which will be
> discussed in the mail accompanying the patch.
> 
> Since I do not use quilt, some patches will inevitably mix several
> modifications, especially the patch to ksrc/skins/posix/syscall.c. I promise,
> next time I will use quilt.
> 
> The result has only be tested in the fast (no syscall) case, to evaluate its
> performance, I will start tests trying to cover the syscall case now, and keep
> you informed.

Central question (IMHO): Do you ensure that the kernel-side atomic op on
 the mutex state can _never_ raise an exception? Or do you have some
atomic mechanism that can handle faults?

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 254 bytes --]

Re: [Xenomai-core] [0/9] Posix skin user-space mutexes

[Gilles Chanteperdrix]

On Thu, Apr 24, 2008 at 9:09 AM, Jan Kiszka <jan.kiszka@domain.hid> wrote:
> Gilles Chanteperdrix wrote:
>  > Hi,
>  >
>  > the patch series to come for review adds support for user-space mutexes to the
>  > posix skin. Since I wanted this support to be available on my AT91RM9200, the
>  > patch series start with patches which are mainly for the ARM architecture, to
>  > end with reimplementation of the kernel-space and user-space mutex services for
>  > the posix skin.
>
>  Cool stuff!
>
>
>  >
>  > Some patches brought some questions when making them, questions which will be
>  > discussed in the mail accompanying the patch.
>  >
>  > Since I do not use quilt, some patches will inevitably mix several
>  > modifications, especially the patch to ksrc/skins/posix/syscall.c. I promise,
>  > next time I will use quilt.
>  >
>  > The result has only be tested in the fast (no syscall) case, to evaluate its
>  > performance, I will start tests trying to cover the syscall case now, and keep
>  > you informed.
>
>  Central question (IMHO): Do you ensure that the kernel-side atomic op on
>   the mutex state can _never_ raise an exception? Or do you have some
>  atomic mechanism that can handle faults?

The atomic op takes place on memory allocated on a shared xnheap. So,
it is basically vmalloc or kmalloced memory in kernel-space and is
hence garanteed to not fault. In user-space, the kernel helper
(kuser_cmpxchg) used for handling atomic_cmpxchg on ARM pre v6 has
builtin support for handling exceptions (basically, the operation is
restarted upon exception). I do not know if it will be the case for
ARM v6 or other arches, but since we are using a mapped heap, I would
expect mlockall to save us.

-- 
 Gilles

Re: [Xenomai-core] [0/9] Posix skin user-space mutexes

[Gilles Chanteperdrix]

On Thu, Apr 24, 2008 at 9:37 AM, Gilles Chanteperdrix
<gilles.chanteperdrix@xenomai.org> wrote:
>
> On Thu, Apr 24, 2008 at 9:09 AM, Jan Kiszka <jan.kiszka@domain.hid> wrote:
>  > Gilles Chanteperdrix wrote:
>  >  > Hi,
>  >  >
>  >  > the patch series to come for review adds support for user-space mutexes to the
>  >  > posix skin. Since I wanted this support to be available on my AT91RM9200, the
>  >  > patch series start with patches which are mainly for the ARM architecture, to
>  >  > end with reimplementation of the kernel-space and user-space mutex services for
>  >  > the posix skin.
>  >
>  >  Cool stuff!
>  >
>  >
>  >  >
>  >  > Some patches brought some questions when making them, questions which will be
>  >  > discussed in the mail accompanying the patch.
>  >  >
>  >  > Since I do not use quilt, some patches will inevitably mix several
>  >  > modifications, especially the patch to ksrc/skins/posix/syscall.c. I promise,
>  >  > next time I will use quilt.
>  >  >
>  >  > The result has only be tested in the fast (no syscall) case, to evaluate its
>  >  > performance, I will start tests trying to cover the syscall case now, and keep
>  >  > you informed.
>  >
>  >  Central question (IMHO): Do you ensure that the kernel-side atomic op on
>  >   the mutex state can _never_ raise an exception? Or do you have some
>  >  atomic mechanism that can handle faults?
>
>  The atomic op takes place on memory allocated on a shared xnheap. So,
>  it is basically vmalloc or kmalloced memory in kernel-space and is
>  hence garanteed to not fault.

Actually, it may be vmalloced memory only on architectures using the
non cached heaps. So, it should be OK on PPC where vmalloc memory
causes problems, because PPC should not need non cached mapping and
will use kmalloced memory.

-- 
 Gilles

Re: [Xenomai-core] [3/9] Define more atomic operations in user-space

[Philippe Gerum]

Gilles Chanteperdrix wrote:
> This patch implements the _read, _set, and _cmpxchg operations on atomic_long_t
> and atomic_ptr_t in user-space in include/asm-generic/atomic.h which should be
> included at the end of include/asm-*/atomic.h after the definition of the same
> operations for the atomic_t type and atomic64_t type on 64 bits platforms.
> 
> These operations are the basic operations used by user-space mutexes. Maybe we
> should add the xnarch_ prefix ?
>

Yes, but more generally, we should rework this to fit the existing atomic
support in include/asm-*/atomic.h, so that we don't end up with sideways to what
has been already designed to support set, get, xchg and the like, in both kernel
and userland context.

Also, unless you find a way to factor in common generic parts among per-arch
atomic ops, I don't see any upside to creating asm-generic/atomic.h, then rely
on arch-dep conditionals to fill them in.

NOTE: the current asm-*/atomic.h support in Xenomai is seldomly used (e.g.
userland only defines the xchg op IIRC), so we may change it globally to fit new
needs should not be that cumbersome.

> ---
>  Makefile.am |    2 -
>  atomic.h    |   68 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 69 insertions(+), 1 deletion(-)
> 
> Index: include/asm-generic/atomic.h
> ===================================================================
> --- include/asm-generic/atomic.h	(revision 0)
> +++ include/asm-generic/atomic.h	(revision 0)
> @@ -0,0 +1,68 @@
> +#ifndef ATOMIC_H
> +#define ATOMIC_H
> +
> +#ifndef __KERNEL__
> +#include <bits/wordsize.h>
> +#if __WORDSIZE == 64
> +typedef atomic64_t atomic_long_t;
> +
> +static inline long atomic_long_read(atomic_long_t *l)
> +{
> +        atomic64_t *v = (atomic64_t *)l;
> +
> +        return (long)atomic64_read(v);
> +}
> +
> +static inline void atomic_long_set(atomic_long_t *l, long i)
> +{
> +        atomic64_t *v = (atomic64_t *)l;
> +
> +        atomic64_set(v, i);
> +}
> +
> +#define atomic_long_cmpxchg(l, old, new) \
> +        (atomic64_cmpxchg((atomic64_t *)(l), (old), (new)))
> +
> +#else /* __WORDSIZE == 32 */
> +typedef atomic_t atomic_long_t;
> +
> +static inline long atomic_long_read(atomic_long_t *l)
> +{
> +        atomic_t *v = (atomic_t *)l;
> +
> +        return (long)atomic_read(v);
> +}
> +
> +static inline void atomic_long_set(atomic_long_t *l, long i)
> +{
> +        atomic_t *v = (atomic_t *)l;
> +
> +        atomic_set(v, i);
> +}
> +
> +#define atomic_long_cmpxchg(l, old, new) \
> +        (atomic_cmpxchg((atomic_t *)(l), (old), (new)))
> +#endif /* __WORDSIZE == 32 */
> +#endif /* __KERNEL__ */
> +
> +typedef atomic_long_t atomic_ptr_t;
> +
> +static inline void *atomic_ptr_read(atomic_ptr_t *l)
> +{
> +        atomic_long_t *v = (atomic_long_t *)l;
> +
> +        return (void *)atomic_long_read(v);
> +}
> +
> +static inline void atomic_ptr_set(atomic_ptr_t *l, void *i)
> +{
> +        atomic_long_t *v = (atomic_long_t *)l;
> +
> +        atomic_long_set(v, (long)i);
> +}
> +
> +#define atomic_ptr_cmpxchg(l, old, new) \
> +        (void *)(atomic_long_cmpxchg((atomic_long_t *)(l), \
> +                                     (long)(old), (long)(new)))
> +
> +#endif /* ATOMIC_H */
> Index: include/asm-generic/Makefile.am
> ===================================================================
> --- include/asm-generic/Makefile.am	(revision 3718)
> +++ include/asm-generic/Makefile.am	(working copy)
> @@ -1,5 +1,5 @@
>  includesubdir = $(includedir)/asm-generic
>  
> -includesub_HEADERS = arith.h features.h hal.h syscall.h system.h wrappers.h
> +includesub_HEADERS = arith.h features.h hal.h syscall.h system.h wrappers.h atomic.h
>  
>  SUBDIRS = bits
> 


-- 
Philippe.

Re: [Xenomai-core] [4/9] Define ARM atomic operations in user-space

[Philippe Gerum]

Gilles Chanteperdrix wrote:
> This patch implements the _read, _set, and _cmpxchg operations on the atomic_t
> type in user-space for ARM, using ldrex/strex on ARM v6, and the Linux kernel
> helper kuser_cmpxchg on ARM pre-v6 (so, without syscalls) without SMP. Only the
> SMP case for pre-v6 ARMs still use syscalls, but this case should not be so
> frequent anyway.
> 

This should probably be reworked along with patch #3. xnarch_* prefix needed in
order to prevent namespace pollution.

> A new macro XNARCH_HAVE_US_ATOMIC_CMPXCHG is defined both in kernel-space and
> user-space, so that the kernel-space can rely on the user-space having
> atomic_cmpxchg defined.
> 
> The plan is to implement these atomic operations for other platforms, defining
> the XNARCH_HAVE_US_ATOMIC_CMPXCHG macro when this is done. When all platforms
> define XNARCH_HAVE_US_ATOMIC_CMPXCGH, we will be able to remove all the
> #ifdefs.
> 
> A question arise about memory barriers. I assumed that atomic_cmpxchg implied a
> barrier, but I am not to sure about that. If that is not the case, we will have
> to implement barriers in user-space as well, and call the in the apropriate
> places in mutexes implementation.
>

I would only assume a compiler barrier with cmpxchg, and likely a memory barrier
in the SMP case. Not much more.

> ---
>  atomic.h |  169 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 166 insertions(+), 3 deletions(-)
> 
> Index: include/asm-arm/atomic.h
> ===================================================================
> --- include/asm-arm/atomic.h	(revision 3718)
> +++ include/asm-arm/atomic.h	(working copy)
> @@ -23,7 +23,6 @@
>  #ifndef _XENO_ASM_ARM_ATOMIC_H
>  #define _XENO_ASM_ARM_ATOMIC_H
>  
> -
>  #ifdef __KERNEL__
>  
>  #include <linux/bitops.h>
> @@ -35,6 +34,8 @@
>  #define xnarch_memory_barrier()  	smp_mb()
>  
>  #if __LINUX_ARM_ARCH__ >= 6
> +#define XNARCH_HAVE_US_ATOMIC_CMPXCHG
> +
>  static inline void atomic_set_mask(unsigned long mask, unsigned long *addr)
>  {
>      unsigned long tmp, tmp2;
> @@ -58,6 +59,11 @@ static inline void atomic_set_mask(unsig
>      *addr |= mask;
>      local_irq_restore_hw(flags);
>  }
> +
> +#ifndef CONFIG_SMP
> +#define XNARCH_HAVE_US_ATOMIC_CMPXCHG
> +#endif /* CONFIG_SMP */
> +
>  #endif /* ARM_ARCH_6 */
>  
>  #define xnarch_atomic_set(pcounter,i)          atomic_set(pcounter,i)
> @@ -75,9 +81,14 @@ typedef atomic_t atomic_counter_t;
>  
>  #include <asm/xenomai/features.h>
>  #include <asm/xenomai/syscall.h>
> +#include <nucleus/compiler.h>
>  
>  typedef struct { volatile int counter; } atomic_counter_t;
>  
> +typedef atomic_counter_t atomic_t;
> +
> +#define atomic_read(v)	((v)->counter)
> +
>  /*
>   * This function doesn't exist, so you'll get a linker error
>   * if something tries to do an invalid xchg().
> @@ -129,6 +140,40 @@ __xchg(volatile void *ptr, unsigned long
>   * Atomic operations lifted from linux/include/asm-arm/atomic.h 
>   */
>  #if CONFIG_XENO_ARM_ARCH >= 6
> +#define XNARCH_HAVE_US_ATOMIC_CMPXCHG
> +
> +static __inline__ void atomic_set(atomic_t *v, int i)
> +{
> +	unsigned long tmp;
> +
> +	__asm__ __volatile__("@ atomic_set\n"
> +"1:	ldrex	%0, [%1]\n"
> +"	strex	%0, %2, [%1]\n"
> +"	teq	%0, #0\n"
> +"	bne	1b"
> +	: "=&r" (tmp)
> +	: "r" (&v->counter), "r" (i)
> +	: "cc");
> +}
> +
> +static __inline__ int atomic_cmpxchg(atomic_t *ptr, int old, int new)
> +{
> +	unsigned long oldval, res;
> +
> +	do {
> +		__asm__ __volatile__("@ atomic_cmpxchg\n"
> +		"ldrex	%1, [%2]\n"
> +		"mov	%0, #0\n"
> +		"teq	%1, %3\n"
> +		"strexeq %0, %4, [%2]\n"
> +		    : "=&r" (res), "=&r" (oldval)
> +		    : "r" (&ptr->counter), "Ir" (old), "r" (new)
> +		    : "cc");
> +	} while (res);
> +
> +	return oldval;
> +}
> +
>  static __inline__ int atomic_add_return(int i, atomic_counter_t *v)
>  {
>      unsigned long tmp;
> @@ -194,7 +239,7 @@ static __inline__ void atomic_clear_mask
>      : "r" (addr), "Ir" (mask)
>      : "cc");
>  }
> -#else /* ARM_ARCH_6 */
> +#elif CONFIG_SMP
>  static __inline__ int atomic_add_return(int i, atomic_counter_t *v)
>  {
>      int ret;
> @@ -224,7 +269,122 @@ static inline void atomic_clear_mask(uns
>      XENOMAI_SYSCALL3(__xn_sys_arch,
>                       XENOMAI_SYSARCH_ATOMIC_CLEAR_MASK, mask, addr);
>  }
> -#endif /* ARM_ARCH_6 */
> +#else /* ARM_ARCH <= 5 && !CONFIG_SMP */
> +#define XNARCH_HAVE_US_ATOMIC_CMPXCHG
> +
> +static __inline__ void atomic_set(atomic_counter_t *ptr, int val)
> +{
> +	ptr->counter = val;
> +}
> +
> +static __inline__ int atomic_cmpxchg(atomic_counter_t *ptr, int old, int new)
> +{
> +        register int asm_old asm("r0") = old;
> +        register int asm_new asm("r1") = new;
> +        register int *asm_ptr asm("r2") = (int *) &ptr->counter;
> +        register int asm_lr asm("lr");
> +	register int asm_tmp asm("r3");
> +
> +	do {
> +		asm volatile ( \
> +			"mov %1, #0xffff0fff\n\t"	\
> +			"mov lr, pc\n\t"		 \
> +			"add pc, %1, #(0xffff0fc0 - 0xffff0fff)\n\t"	\
> +			: "+r"(asm_old), "=&r"(asm_tmp), "=r"(asm_lr)	\
> +			: "r"(asm_new), "r"(asm_ptr) \
> +			: "ip", "cc", "memory");
> +		if (likely(!asm_old))
> +			return old;
> +	} while ((asm_old = *asm_ptr) == old);
> +        return asm_old;
> +}
> +
> +static __inline__ int atomic_add_return(int i, atomic_counter_t *v)
> +{
> +	register int asm_old asm("r0");
> +	register int asm_new asm("r1");
> +	register int *asm_ptr asm("r2") = (int *) &v->counter;
> +        register int asm_lr asm("lr");
> +	register int asm_tmp asm("r3");
> +
> +	asm volatile ( \
> +		"1: @ atomic_add\n\t" \
> +		"ldr	%0, [%4]\n\t" \
> +		"mov	%1, #0xffff0fff\n\t" \
> +		"add	lr, pc, #4\n\t" \
> +		"add	%3, %0, %5\n\t"\
> +		"add	pc, %1, #(0xffff0fc0 - 0xffff0fff)\n\t" \
> +		"bcc	1b" \
> +		: "=&r" (asm_old), "=&r"(asm_tmp), "=r"(asm_lr), "=r"(asm_new) \
> +		: "r" (asm_ptr), "rIL"(i) \
> +		: "ip", "cc", "memory");
> +	return asm_new;
> +}
> +
> +static __inline__ int atomic_sub_return(int i, atomic_counter_t *v)
> +{
> +	register int asm_old asm("r0");
> +	register int asm_new asm("r1");
> +	register int *asm_ptr asm("r2") = (int *) &v->counter;
> +        register int asm_lr asm("lr");
> +	register int asm_tmp asm("r3");
> +
> +	asm volatile ( \
> +		"1: @ atomic_sub\n\t" \
> +		"ldr	%0, [%4]\n\t" \
> +		"mov	%1, #0xffff0fff\n\t" \
> +		"add	lr, pc, #4\n\t" \
> +		"sub	%3, %0, %5\n\t"\
> +		"add	pc, %1, #(0xffff0fc0 - 0xffff0fff)\n\t" \
> +		"bcc	1b" \
> +		: "=&r" (asm_old), "=&r"(asm_tmp), "=r"(asm_lr), "=r"(asm_new) \
> +		: "r" (asm_ptr), "rIL"(i) \
> +		: "ip", "cc", "memory");
> +	return asm_new;
> +}
> +
> +static __inline__ void atomic_set_mask(long mask, atomic_counter_t *v)
> +{
> +	register int asm_old asm("r0");
> +	register int asm_new asm("r1");
> +	register int *asm_ptr asm("r2") = (int *) &v->counter;
> +        register int asm_lr asm("lr");
> +	register int asm_tmp asm("r3");
> +
> +	asm volatile ( \
> +		"1: @ atomic_set_mask\n\t" \
> +		"ldr	%0, [%4]\n\t" \
> +		"mov	%1, #0xffff0fff\n\t" \
> +		"add	lr, pc, #4\n\t" \
> +		"orr	%3, %0, %5\n\t"\
> +		"add	pc, %1, #(0xffff0fc0 - 0xffff0fff)\n\t" \
> +		"bcc	1b" \
> +		: "=&r" (asm_old), "=&r"(asm_tmp), "=r"(asm_lr), "=r"(asm_new) \
> +		: "r" (asm_ptr), "rIL"(mask) \
> +		: "ip", "cc", "memory");
> +}
> +
> +static __inline__ void atomic_clear_mask(long mask, atomic_counter_t *v)
> +{
> +	register int asm_old asm("r0");
> +	register int asm_new asm("r1");
> +	register int *asm_ptr asm("r2") = (int *) &v->counter;
> +        register int asm_lr asm("lr");
> +	register int asm_tmp asm("r3");
> +
> +	asm volatile ( \
> +		"1: @ atomic_clear_mask\n\t" \
> +		"ldr	%0, [%4]\n\t" \
> +		"mov	%1, #0xffff0fff\n\t" \
> +		"add	lr, pc, #4\n\t" \
> +		"bic	%3, %0, %5\n\t" \
> +		"add	pc, %1, #(0xffff0fc0 - 0xffff0fff)\n\t" \
> +		"bcc	1b" \
> +		: "=&r" (asm_old), "=&r"(asm_tmp), "=r"(asm_lr), "=r"(asm_new) \
> +		: "r" (asm_ptr), "rIL"(mask) \
> +		: "ip", "cc", "memory");
> +}
> +#endif /* ARM_ARCH <= 5 && !CONFIG_SMP */
>  
>  #define xnarch_memory_barrier()                 __asm__ __volatile__("": : :"memory")
>  
> @@ -241,6 +401,9 @@ static inline void atomic_clear_mask(uns
>  
>  typedef unsigned long atomic_flags_t;
>  
> +/* Add support for atomic_long_t and atomic_ptr_t */
> +#include <asm-generic/xenomai/atomic.h>
> +
>  #endif /* !_XENO_ASM_ARM_ATOMIC_H */
>  
>  // vim: ts=4 et sw=4 sts=4
> 


-- 
Philippe.

Re: [Xenomai-core] [5/9] Define new syscalls for the posix skin

[Philippe Gerum]

Gilles Chanteperdrix wrote:
> This patch defines the new syscalls get_heap_addr (more on this syscall later),
> and get_thread_cb. The get_thread_cb syscall is used in user-space to associate
> the thread xnthread_t pointer with each user-space xenomai thread, using POSIX
> TSD.
> 
> This is needed for the mutex implementation to allow setting the mutex owner.
> 
> However, two things: 
> - I do not know if you should like that much passing kernel-space pointers to
>   user-space;

We already did that in a few occasions. Let's consider this as an opaque handle
since those values belong to a different address space.

> - this is implemented as a posix skin service, but should probably be defined as
>   a core syscall, and called in all skin implementations, so as to allow threads
>   from other skins to user Xenomai posix skin services.
> 

Since you are doing the initial effort and designing the interface, please move
this to the nucleus directly. If the first implementation only goes through the
POSIX skin, it will likely lower the incentive for other skins to use that
feature, since the amount of work to integrate it will be higher.

> ---
>  include/posix/syscall.h  |    3 +++
>  src/skins/posix/thread.c |   11 +++++++++++
>  2 files changed, 14 insertions(+)
> 
> Index: include/posix/syscall.h
> ===================================================================
> --- include/posix/syscall.h	(revision 3718)
> +++ include/posix/syscall.h	(working copy)
> @@ -46,6 +46,7 @@
>  #define __pse51_mutex_lock            20
>  #define __pse51_mutex_timedlock       21
>  #define __pse51_mutex_trylock         22
> +#define __pse51_check_init            __pse51_mutex_trylock
>  #define __pse51_mutex_unlock          23
>  #define __pse51_cond_init             24
>  #define __pse51_cond_destroy          25
> @@ -101,6 +102,8 @@
>  #define __pse51_thread_getschedparam  75
>  #define __pse51_thread_kill           76
>  #define __pse51_select                77
> +#define __pse51_get_heap_addr         78
> +#define __pse51_get_thread_cb	      79
>  
>  #ifdef __KERNEL__
>  
> Index: src/skins/posix/thread.c
> ===================================================================
> --- src/skins/posix/thread.c	(revision 3718)
> +++ src/skins/posix/thread.c	(working copy)
> @@ -26,6 +26,7 @@
>  #include <posix/syscall.h>
>  
>  extern int __pse51_muxid;
> +extern pthread_key_t pse51_cur_key;
>  
>  static pthread_attr_t default_attr;
>  static int linuxthreads;
> @@ -41,6 +42,13 @@ static void __pthread_sigharden_handler(
>  	XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_XENO_DOMAIN);
>  }
>  
> +static void set_kthread_cb(void)
> +{
> +	void *kthread_cb;
> +	XENOMAI_SKINCALL1(__pse51_muxid, __pse51_get_thread_cb, &kthread_cb);
> +	pthread_setspecific(pse51_cur_key, kthread_cb);
> +}
> +
>  int __wrap_pthread_setschedparam(pthread_t thread,
>  				 int policy, const struct sched_param *param)
>  {
> @@ -56,6 +64,7 @@ int __wrap_pthread_setschedparam(pthread
>  
>  	if (!err && promoted) {
>  		old_sigharden_handler = signal(SIGHARDEN, &__pthread_sigharden_handler);
> +		set_kthread_cb();
>  		if (policy != SCHED_OTHER)
>  			XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_XENO_DOMAIN);
>  	}
> @@ -132,6 +141,8 @@ static void *__pthread_trampoline(void *
>  	start = iargs->start;
>  	cookie = iargs->arg;
>  
> +	set_kthread_cb();
> +
>  	__real_sem_post(&iargs->sync);
>  
>  	if (!err) {
> 
> 


-- 
Philippe.

Re: [Xenomai-core] [6/9] Rework posix skin shared heaps support, add per-process shared heap.

[Philippe Gerum]

Gilles Chanteperdrix wrote:
> This patch removes much of the #ifdefery that was in ksrc/skins/posix/shm.c. It
> also initializes a global shared heap (for process-shared objects), as well as a
> per-process shared heap. These two heaps are mapped in the posix skin library
> initialization, using the new get_heap_addr syscall to get their addresses.
> 
> Of course, at this chance, we use the XNARCH_SHARED_HEAP_FLAGS macro, as a flag
> passed to xnheap_init_mapped.
> 
> Several questions here:
> - the heaps are created with a size of three pages, hardcoded, maybe we should
>   make this size somehow configurable (Kconfig, /proc, whatever). My idea was
>   that each mutex will use about 4 bytes in a heap, so 12 Kbytes is plenty.

This is clearly a good candidate for Kconfig. Tuning this dynamically through
/proc would make no sense: either your setup can work at boot time, or it
cannot, in which case you have to fix the configuration.

> - if we do not want to waste memory, like the xnthread_t * TSD, I wonder if
>   these two heaps should not be allocated by the core event callback,
>   unfortunately, this would mean that we would have to associate a per-process
>   data with the core skin, which we avoided until now.

Thinking ahead, this might not be the last time we would have to add some
context to the core skin members.

> 
> ---
>  ksrc/skins/posix/internal.h |   10 ++++++
>  ksrc/skins/posix/module.c   |   41 ++++++++++++++++++++++++++
>  ksrc/skins/posix/shm.c      |   44 ++++------------------------
>  src/skins/posix/init.c      |   68 ++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 126 insertions(+), 37 deletions(-)
> 
> Index: ksrc/skins/posix/internal.h
> ===================================================================
> --- ksrc/skins/posix/internal.h	(revision 3718)
> +++ ksrc/skins/posix/internal.h	(working copy)
> @@ -71,6 +71,7 @@ typedef struct {
>  	xnqueue_t semq;
>  	xnqueue_t threadq;
>  	xnqueue_t timerq;
> +	xnheap_t shm_heap;
>  } pse51_kqueues_t;
>  
>  #ifdef CONFIG_XENO_OPT_PERVASIVE
> @@ -202,4 +203,13 @@ static inline int clock_flag(int flag, c
>  int pse51_mq_select_bind(mqd_t fd, struct xnselector *selector,
>  			 unsigned type, unsigned index);
>  
> +#ifdef CONFIG_XENO_OPT_PERVASIVE
> +#define pse51_shm_heap_init(h, l, f) xnheap_init_mapped(h, l, f)
> +#define pse51_shm_heap_destroy(h) xnheap_destroy_mapped(h)
> +#else /* !CONFIG_XENO_OPT_PERVASIVE */
> +int pse51_shm_heap_init(xnheap_t *heap, unsigned len, int flags);
> +
> +void pse51_shm_heap_destroy(xnheap_t *heap);
> +#endif /* !CONFIG_XENO_OPT_PERVASIVE */
> +
>  #endif /* !_POSIX_INTERNAL_H */
> Index: ksrc/skins/posix/module.c
> ===================================================================
> --- ksrc/skins/posix/module.c	(revision 3718)
> +++ ksrc/skins/posix/module.c	(working copy)
> @@ -78,6 +78,36 @@ MODULE_PARM_DESC(time_slice, "Default ti
>  
>  xntbase_t *pse51_tbase;
>  
> +#ifndef CONFIG_XENO_OPT_PERVASIVE
> +static void pse51_free_heap_extent(xnheap_t *heap,
> +				   void *extent, u_long size, void *cookie)
> +{
> +	xnarch_free_host_mem(extent, size);
> +}
> +
> +int pse51_shm_heap_init(xnheap_t *heap, unsigned len, int flags)
> +{
> +	void *heapaddr = xnarch_alloc_host_mem(len);
> +	int err;
> +
> +	if (heapaddr) {
> +		err = xnheap_init(&shm->heapbase,
> +				  heapaddr, len, XNCORE_PAGE_SIZE);
> +		if (err)
> +			xnarch_free_host_mem(heapaddr, len);
> +
> +		return err;
> +	}
> +
> +	return -ENOMEM;
> +}
> +
> +void pse51_shm_heap_destroy(xnheap_t *heap)
> +{
> +	xnheap_destroy(heap, &pse51_free_heap_extent, NULL);
> +}
> +#endif /* !CONFIG_XENO_OPT_PERVASIVE */
> +
>  static void pse51_shutdown(int xtype)
>  {
>  	pse51_thread_pkg_cleanup();
> @@ -98,6 +128,7 @@ static void pse51_shutdown(int xtype)
>  #ifdef CONFIG_XENO_OPT_PERVASIVE
>  	pse51_syscall_cleanup();
>  #endif /* CONFIG_XENO_OPT_PERVASIVE */
> +	pse51_shm_heap_destroy(&pse51_global_kqueues.shm_heap);
>  #ifdef __KERNEL__
>  	pse51_apc_pkg_cleanup();
>  #endif /* __KERNEL__ */
> @@ -127,11 +158,21 @@ int SKIN_INIT(posix)
>  		goto fail_free_tbase;
>  #endif /* __KERNEL__ */
>  
> +	err = pse51_shm_heap_init(&pse51_global_kqueues.shm_heap, 3 * PAGE_SIZE,
> +				  (XNARCH_SHARED_HEAP_FLAGS ?: GFP_USER));
> +	if (err)
> +		goto fail_cleanup_apc;
> +	
>  #ifdef CONFIG_XENO_OPT_PERVASIVE
>  	err = pse51_syscall_init();
>  #endif /* CONFIG_XENO_OPT_PERVASIVE */
>  	if (err != 0) {
> +		pse51_shm_heap_destroy(&pse51_global_kqueues.shm_heap);
> +	  fail_cleanup_apc:
> +#ifdef __KERNEL__
> +		pse51_apc_pkg_cleanup();
>  	  fail_free_tbase:
> +#endif /* __KERNEL__ */
>  		xntbase_free(pse51_tbase);
>  	fail_shutdown_pod:
>  		xnpod_shutdown(err);
> Index: ksrc/skins/posix/shm.c
> ===================================================================
> --- ksrc/skins/posix/shm.c	(revision 3718)
> +++ ksrc/skins/posix/shm.c	(working copy)
> @@ -90,14 +90,6 @@ static void pse51_shm_init(pse51_shm_t *
>  	appendq(&pse51_shmq, &shm->link);
>  }
>  
> -#ifndef CONFIG_XENO_OPT_PERVASIVE
> -static void pse51_free_heap_extent(xnheap_t *heap,
> -				   void *extent, u_long size, void *cookie)
> -{
> -	xnarch_free_host_mem(extent, size);
> -}
> -#endif /* !CONFIG_XENO_OPT_PERVASIVE */
> -
>  /* Must be called nklock locked, irq off. */
>  static void pse51_shm_destroy(pse51_shm_t * shm, int force)
>  {
> @@ -111,11 +103,7 @@ static void pse51_shm_destroy(pse51_shm_
>  	if (shm->addr) {
>  		xnheap_free(&shm->heapbase, shm->addr);
>  
> -#ifdef CONFIG_XENO_OPT_PERVASIVE
> -		xnheap_destroy_mapped(&shm->heapbase);
> -#else /* !CONFIG_XENO_OPT_PERVASIVE. */
> -		xnheap_destroy(&shm->heapbase, &pse51_free_heap_extent, NULL);
> -#endif /* !CONFIG_XENO_OPT_PERVASIVE. */
> +		pse51_shm_heap_destroy(&shm->heapbase);
>  
>  		shm->addr = NULL;
>  		shm->size = 0;
> @@ -534,37 +522,19 @@ int ftruncate(int fd, off_t len)
>  			memcpy(addr, shm->addr, size);
>  
>  			xnheap_free(&shm->heapbase, shm->addr);
> -#ifdef CONFIG_XENO_OPT_PERVASIVE
> -			xnheap_destroy_mapped(&shm->heapbase);
> -#else /* !CONFIG_XENO_OPT_PERVASIVE. */
> -			xnheap_destroy(&shm->heapbase, &pse51_free_heap_extent,
> -				       NULL);
> -#endif /* !CONFIG_XENO_OPT_PERVASIVE. */
> +			pse51_shm_heap_destroy(&shm->heapbase);
>  
>  			shm->addr = NULL;
>  			shm->size = 0;
>  		}
>  
>  		if (len) {
> -#ifdef CONFIG_XENO_OPT_PERVASIVE
> -			int flags = len <= 128 * 1024 ? GFP_USER : 0;
> -			err = -xnheap_init_mapped(&shm->heapbase, len, flags);
> -#else /* !CONFIG_XENO_OPT_PERVASIVE. */
> -			{
> -				void *heapaddr = xnarch_alloc_host_mem(len);
> -
> -				if (heapaddr)
> -					err =
> -					    -xnheap_init(&shm->heapbase,
> -							 heapaddr, len,
> -							 XNCORE_PAGE_SIZE);
> -				else
> -					err = ENOMEM;
> +			int flags = (XNARCH_SHARED_HEAP_FLAGS ?:
> +				     len <= 128 * 1024 ? GFP_USER : 0);
>  
> -				if (err)
> -					goto err_up;
> -			}
> -#endif /* !CONFIG_XENO_OPT_PERVASIVE. */
> +			err = -pse51_shm_heap_init(&shm->heapbase, len, flags);
> +			if (err)
> +				goto err_up;
>  
>  			shm->size = xnheap_max_contiguous(&shm->heapbase);
>  			shm->addr = xnheap_alloc(&shm->heapbase, shm->size);
> Index: src/skins/posix/init.c
> ===================================================================
> --- src/skins/posix/init.c	(revision 3718)
> +++ src/skins/posix/init.c	(working copy)
> @@ -33,10 +33,49 @@ int __pse51_muxid = -1;
>  int __rtdm_muxid = -1;
>  int __rtdm_fd_start = INT_MAX;
>  static int fork_handler_registered;
> +pthread_key_t pse51_cur_key;
> +unsigned long pse51_shared_map[2] = {0, 0};
>  
>  int __wrap_pthread_setschedparam(pthread_t, int, const struct sched_param *);
>  void pse51_clock_init(int);
>  
> +static void *map_shared_heap(unsigned shared)
> +{
> +	void *heap_addr;
> +	int fd, err;
> +
> +	fd = __real_open("/dev/rtheap", O_RDWR, 0);
> +	if (fd < 0) {
> +		fprintf(stderr, "open: %m\n");
> +		return MAP_FAILED;
> +	}
> +
> +	err = XENOMAI_SKINCALL2(__pse51_muxid,
> +				__pse51_get_heap_addr, &heap_addr, shared);
> +	if (err < 0) {
> +		fprintf(stderr, "get_heap_addr: %m\n");
> +		return MAP_FAILED;
> +	}
> +
> +	err = __real_ioctl(fd, 0, heap_addr);
> +	if (err < 0) {
> +		fprintf(stderr, "ioctl: %m\n");
> +		return MAP_FAILED;
> +	}
> +
> +	heap_addr = __real_mmap(NULL, 3 * getpagesize(),
> +				PROT_READ | PROT_WRITE,
> +				MAP_SHARED, fd, 0);
> +	close(fd);
> +
> +	return heap_addr;
> +}
> +
> +static void unmap_shared_heap(unsigned long heap_addr)
> +{
> +	__real_munmap((void *) heap_addr, 3 * getpagesize());
> +}
> +
>  static __attribute__ ((constructor))
>  void __init_posix_interface(void)
>  {
> @@ -83,7 +122,35 @@ void __init_posix_interface(void)
>  	}
>  #endif /* !CONFIG_XENO_POSIX_AUTO_MLOCKALL */
>  
> +	if (pse51_shared_map[0])
> +		/* We have forked, so, let us recreate the private mapping */
> +		unmap_shared_heap(pse51_shared_map[0]);
> +	pse51_shared_map[0] = (unsigned long) map_shared_heap(0);
> +	if (pse51_shared_map[0] == (unsigned long) MAP_FAILED) {
> +		perror("Xenomai Posix skin init: mmap(local heap)");
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	fprintf(stderr, "sizeof(pthread_mutex_t): %d,"
> +		" sizeof(shadow_mutex): %d\n",
> +		sizeof(pthread_mutex_t), sizeof(struct __shadow_mutex));
> +	if (sizeof(struct __shadow_mutex) > sizeof(pthread_mutex_t))
> +		exit(EXIT_FAILURE);
> +	
>  	if (!fork_handler_registered) {
> +		err = pthread_key_create(&pse51_cur_key, NULL);
> +		if (err) {
> +			fprintf(stderr, "Xenomai Posix skin init:"
> +				" pthread_key_create: %s\n", strerror(err));
> +			exit(EXIT_FAILURE);
> +		}
> +
> +		pse51_shared_map[1] = (unsigned long) map_shared_heap(1);
> +		if (pse51_shared_map[1] == (unsigned long) MAP_FAILED) {
> +			perror("Xenomai Posix skin init: mmap(global heap)");
> +			exit(EXIT_FAILURE);
> +		}
> +	
>  		err = pthread_atfork(NULL, NULL, &__init_posix_interface);
>  		if (err) {
>  			fprintf(stderr, "Xenomai Posix skin init: "
> @@ -93,3 +160,4 @@ void __init_posix_interface(void)
>  		fork_handler_registered = 1;
>  	}
>  }
> +
> 
> 


-- 
Philippe.

Re: [Xenomai-core] [3/9] Define more atomic operations in user-space

[Gilles Chanteperdrix]

On Fri, Apr 25, 2008 at 9:48 AM, Philippe Gerum <rpm@xenomai.org> wrote:
> Gilles Chanteperdrix wrote:
>  > This patch implements the _read, _set, and _cmpxchg operations on atomic_long_t
>  > and atomic_ptr_t in user-space in include/asm-generic/atomic.h which should be
>  > included at the end of include/asm-*/atomic.h after the definition of the same
>  > operations for the atomic_t type and atomic64_t type on 64 bits platforms.
>  >
>  > These operations are the basic operations used by user-space mutexes. Maybe we
>  > should add the xnarch_ prefix ?
>  >
>
>  Yes, but more generally, we should rework this to fit the existing atomic
>  support in include/asm-*/atomic.h, so that we don't end up with sideways to what
>  has been already designed to support set, get, xchg and the like, in both kernel
>  and userland context.

That is not exactly sideways... Linux include/asm-generic/atomic.h
defines operations for atomic_long_t. This file adds two things:
- support for atomic_long_t in user-space (where we can not include
linux include/asm-generic/atomic.h)
- support for a new type atomic_ptr_t both to kernel-space and
user-space, the aim is to avoid all the casts that would take place if
we wanted to use atomic_long_t to store pointers.

However for this file to work, it has to be included by asm-*/atomic.h
after the definition of atomic_t (and atomic64_t on 64 bits
platforms). So linux includes asm-generic/atomic.h at the end of
asm/atomic.h, I simply reproduced this scheme with Xenomai
include/asm-*/atomic.h.

-- 
 Gilles

Re: [Xenomai-core] [3/9] Define more atomic operations in user-space

[Philippe Gerum]

Gilles Chanteperdrix wrote:
> On Fri, Apr 25, 2008 at 9:48 AM, Philippe Gerum <rpm@xenomai.org> wrote:
>> Gilles Chanteperdrix wrote:
>>  > This patch implements the _read, _set, and _cmpxchg operations on atomic_long_t
>>  > and atomic_ptr_t in user-space in include/asm-generic/atomic.h which should be
>>  > included at the end of include/asm-*/atomic.h after the definition of the same
>>  > operations for the atomic_t type and atomic64_t type on 64 bits platforms.
>>  >
>>  > These operations are the basic operations used by user-space mutexes. Maybe we
>>  > should add the xnarch_ prefix ?
>>  >
>>
>>  Yes, but more generally, we should rework this to fit the existing atomic
>>  support in include/asm-*/atomic.h, so that we don't end up with sideways to what
>>  has been already designed to support set, get, xchg and the like, in both kernel
>>  and userland context.
> 
> That is not exactly sideways... Linux include/asm-generic/atomic.h
> defines operations for atomic_long_t. This file adds two things:
> - support for atomic_long_t in user-space (where we can not include
> linux include/asm-generic/atomic.h)
> - support for a new type atomic_ptr_t both to kernel-space and
> user-space, the aim is to avoid all the casts that would take place if
> we wanted to use atomic_long_t to store pointers.
> 
> However for this file to work, it has to be included by asm-*/atomic.h
> after the definition of atomic_t (and atomic64_t on 64 bits
> platforms). So linux includes asm-generic/atomic.h at the end of
> asm/atomic.h, I simply reproduced this scheme with Xenomai
> include/asm-*/atomic.h.
> 

Focusing on user-space: 1) xnarch_atomic_xchg is meant to work on long types; 2)
set, get routines are not defined in that scope. If the purpose is to define
integer-type ops to handle pointer-type data atomically (i.e. intptr_t), then I
would rather check whether we actually need non-long support at all in
user-space. In case we don't, I would simply reply on the existing
implementation of asm-*/atomic.h + the set / get extensions you provide.

-- 
Philippe.

Re: [Xenomai-core] [3/9] Define more atomic operations in user-space

[Gilles Chanteperdrix]

On Fri, Apr 25, 2008 at 3:42 PM, Philippe Gerum <rpm@xenomai.org> wrote:
>
> Gilles Chanteperdrix wrote:
>  > On Fri, Apr 25, 2008 at 9:48 AM, Philippe Gerum <rpm@xenomai.org> wrote:
>  >> Gilles Chanteperdrix wrote:
>  >>  > This patch implements the _read, _set, and _cmpxchg operations on atomic_long_t
>  >>  > and atomic_ptr_t in user-space in include/asm-generic/atomic.h which should be
>  >>  > included at the end of include/asm-*/atomic.h after the definition of the same
>  >>  > operations for the atomic_t type and atomic64_t type on 64 bits platforms.
>  >>  >
>  >>  > These operations are the basic operations used by user-space mutexes. Maybe we
>  >>  > should add the xnarch_ prefix ?
>  >>  >
>  >>
>  >>  Yes, but more generally, we should rework this to fit the existing atomic
>  >>  support in include/asm-*/atomic.h, so that we don't end up with sideways to what
>  >>  has been already designed to support set, get, xchg and the like, in both kernel
>  >>  and userland context.
>  >
>  > That is not exactly sideways... Linux include/asm-generic/atomic.h
>  > defines operations for atomic_long_t. This file adds two things:
>  > - support for atomic_long_t in user-space (where we can not include
>  > linux include/asm-generic/atomic.h)
>  > - support for a new type atomic_ptr_t both to kernel-space and
>  > user-space, the aim is to avoid all the casts that would take place if
>  > we wanted to use atomic_long_t to store pointers.
>  >
>  > However for this file to work, it has to be included by asm-*/atomic.h
>  > after the definition of atomic_t (and atomic64_t on 64 bits
>  > platforms). So linux includes asm-generic/atomic.h at the end of
>  > asm/atomic.h, I simply reproduced this scheme with Xenomai
>  > include/asm-*/atomic.h.
>  >
>
>  Focusing on user-space: 1) xnarch_atomic_xchg is meant to work on long types; 2)
>  set, get routines are not defined in that scope. If the purpose is to define
>  integer-type ops to handle pointer-type data atomically (i.e. intptr_t), then I
>  would rather check whether we actually need non-long support at all in
>  user-space. In case we don't, I would simply reply on the existing
>  implementation of asm-*/atomic.h + the set / get extensions you provide.

I use both atomic_t and atomic_ptr_t for the implementation of
user-space mutexes. The problem is that I am constrained by the size
of pthread_mutex_t, so the "control block read-write locks"
implementation use atomic_t.

-- 
 Gilles

Re: [Xenomai-core] [3/9] Define more atomic operations in user-space

[Philippe Gerum]

Gilles Chanteperdrix wrote:
> On Fri, Apr 25, 2008 at 3:42 PM, Philippe Gerum <rpm@xenomai.org> wrote:
>> Gilles Chanteperdrix wrote:
>>  > On Fri, Apr 25, 2008 at 9:48 AM, Philippe Gerum <rpm@xenomai.org> wrote:
>>  >> Gilles Chanteperdrix wrote:
>>  >>  > This patch implements the _read, _set, and _cmpxchg operations on atomic_long_t
>>  >>  > and atomic_ptr_t in user-space in include/asm-generic/atomic.h which should be
>>  >>  > included at the end of include/asm-*/atomic.h after the definition of the same
>>  >>  > operations for the atomic_t type and atomic64_t type on 64 bits platforms.
>>  >>  >
>>  >>  > These operations are the basic operations used by user-space mutexes. Maybe we
>>  >>  > should add the xnarch_ prefix ?
>>  >>  >
>>  >>
>>  >>  Yes, but more generally, we should rework this to fit the existing atomic
>>  >>  support in include/asm-*/atomic.h, so that we don't end up with sideways to what
>>  >>  has been already designed to support set, get, xchg and the like, in both kernel
>>  >>  and userland context.
>>  >
>>  > That is not exactly sideways... Linux include/asm-generic/atomic.h
>>  > defines operations for atomic_long_t. This file adds two things:
>>  > - support for atomic_long_t in user-space (where we can not include
>>  > linux include/asm-generic/atomic.h)
>>  > - support for a new type atomic_ptr_t both to kernel-space and
>>  > user-space, the aim is to avoid all the casts that would take place if
>>  > we wanted to use atomic_long_t to store pointers.
>>  >
>>  > However for this file to work, it has to be included by asm-*/atomic.h
>>  > after the definition of atomic_t (and atomic64_t on 64 bits
>>  > platforms). So linux includes asm-generic/atomic.h at the end of
>>  > asm/atomic.h, I simply reproduced this scheme with Xenomai
>>  > include/asm-*/atomic.h.
>>  >
>>
>>  Focusing on user-space: 1) xnarch_atomic_xchg is meant to work on long types; 2)
>>  set, get routines are not defined in that scope. If the purpose is to define
>>  integer-type ops to handle pointer-type data atomically (i.e. intptr_t), then I
>>  would rather check whether we actually need non-long support at all in
>>  user-space. In case we don't, I would simply reply on the existing
>>  implementation of asm-*/atomic.h + the set / get extensions you provide.
> 
> I use both atomic_t and atomic_ptr_t for the implementation of
> user-space mutexes. The problem is that I am constrained by the size
> of pthread_mutex_t, so the "control block read-write locks"
> implementation use atomic_t.
> 

Ok, makes sense. Let's just fix the namespace then, so that we don't get the
feeling of having duplicate sets of operations.

-- 
Philippe.

Re: [Xenomai-core] [3/9] Define more atomic operations in user-space

[Gilles Chanteperdrix]

On Fri, Apr 25, 2008 at 4:01 PM, Philippe Gerum <rpm@xenomai.org> wrote:
>
> Gilles Chanteperdrix wrote:
>  > On Fri, Apr 25, 2008 at 3:42 PM, Philippe Gerum <rpm@xenomai.org> wrote:
>  >> Gilles Chanteperdrix wrote:
>  >>  > On Fri, Apr 25, 2008 at 9:48 AM, Philippe Gerum <rpm@xenomai.org> wrote:
>  >>  >> Gilles Chanteperdrix wrote:
>  >>  >>  > This patch implements the _read, _set, and _cmpxchg operations on atomic_long_t
>  >>  >>  > and atomic_ptr_t in user-space in include/asm-generic/atomic.h which should be
>  >>  >>  > included at the end of include/asm-*/atomic.h after the definition of the same
>  >>  >>  > operations for the atomic_t type and atomic64_t type on 64 bits platforms.
>  >>  >>  >
>  >>  >>  > These operations are the basic operations used by user-space mutexes. Maybe we
>  >>  >>  > should add the xnarch_ prefix ?
>  >>  >>  >
>  >>  >>
>  >>  >>  Yes, but more generally, we should rework this to fit the existing atomic
>  >>  >>  support in include/asm-*/atomic.h, so that we don't end up with sideways to what
>  >>  >>  has been already designed to support set, get, xchg and the like, in both kernel
>  >>  >>  and userland context.
>  >>  >
>  >>  > That is not exactly sideways... Linux include/asm-generic/atomic.h
>  >>  > defines operations for atomic_long_t. This file adds two things:
>  >>  > - support for atomic_long_t in user-space (where we can not include
>  >>  > linux include/asm-generic/atomic.h)
>  >>  > - support for a new type atomic_ptr_t both to kernel-space and
>  >>  > user-space, the aim is to avoid all the casts that would take place if
>  >>  > we wanted to use atomic_long_t to store pointers.
>  >>  >
>  >>  > However for this file to work, it has to be included by asm-*/atomic.h
>  >>  > after the definition of atomic_t (and atomic64_t on 64 bits
>  >>  > platforms). So linux includes asm-generic/atomic.h at the end of
>  >>  > asm/atomic.h, I simply reproduced this scheme with Xenomai
>  >>  > include/asm-*/atomic.h.
>  >>  >
>  >>
>  >>  Focusing on user-space: 1) xnarch_atomic_xchg is meant to work on long types; 2)
>  >>  set, get routines are not defined in that scope. If the purpose is to define
>  >>  integer-type ops to handle pointer-type data atomically (i.e. intptr_t), then I
>  >>  would rather check whether we actually need non-long support at all in
>  >>  user-space. In case we don't, I would simply reply on the existing
>  >>  implementation of asm-*/atomic.h + the set / get extensions you provide.
>  >
>  > I use both atomic_t and atomic_ptr_t for the implementation of
>  > user-space mutexes. The problem is that I am constrained by the size
>  > of pthread_mutex_t, so the "control block read-write locks"
>  > implementation use atomic_t.
>  >
>
>  Ok, makes sense. Let's just fix the namespace then, so that we don't get the
>  feeling of having duplicate sets of operations.

That said, there is just enough room for replacing the atomic_t with
an atomic_long_t. So, we can make xnarch_atomic_t a long type. Have
you anything agains making an xnarch_atomic_ptr_t ?

-- 
 Gilles

Re: [Xenomai-core] [3/9] Define more atomic operations in user-space

[Philippe Gerum]

Gilles Chanteperdrix wrote:
> On Fri, Apr 25, 2008 at 4:01 PM, Philippe Gerum <rpm@xenomai.org> wrote:
>> Gilles Chanteperdrix wrote:
>>  > On Fri, Apr 25, 2008 at 3:42 PM, Philippe Gerum <rpm@xenomai.org> wrote:
>>  >> Gilles Chanteperdrix wrote:
>>  >>  > On Fri, Apr 25, 2008 at 9:48 AM, Philippe Gerum <rpm@xenomai.org> wrote:
>>  >>  >> Gilles Chanteperdrix wrote:
>>  >>  >>  > This patch implements the _read, _set, and _cmpxchg operations on atomic_long_t
>>  >>  >>  > and atomic_ptr_t in user-space in include/asm-generic/atomic.h which should be
>>  >>  >>  > included at the end of include/asm-*/atomic.h after the definition of the same
>>  >>  >>  > operations for the atomic_t type and atomic64_t type on 64 bits platforms.
>>  >>  >>  >
>>  >>  >>  > These operations are the basic operations used by user-space mutexes. Maybe we
>>  >>  >>  > should add the xnarch_ prefix ?
>>  >>  >>  >
>>  >>  >>
>>  >>  >>  Yes, but more generally, we should rework this to fit the existing atomic
>>  >>  >>  support in include/asm-*/atomic.h, so that we don't end up with sideways to what
>>  >>  >>  has been already designed to support set, get, xchg and the like, in both kernel
>>  >>  >>  and userland context.
>>  >>  >
>>  >>  > That is not exactly sideways... Linux include/asm-generic/atomic.h
>>  >>  > defines operations for atomic_long_t. This file adds two things:
>>  >>  > - support for atomic_long_t in user-space (where we can not include
>>  >>  > linux include/asm-generic/atomic.h)
>>  >>  > - support for a new type atomic_ptr_t both to kernel-space and
>>  >>  > user-space, the aim is to avoid all the casts that would take place if
>>  >>  > we wanted to use atomic_long_t to store pointers.
>>  >>  >
>>  >>  > However for this file to work, it has to be included by asm-*/atomic.h
>>  >>  > after the definition of atomic_t (and atomic64_t on 64 bits
>>  >>  > platforms). So linux includes asm-generic/atomic.h at the end of
>>  >>  > asm/atomic.h, I simply reproduced this scheme with Xenomai
>>  >>  > include/asm-*/atomic.h.
>>  >>  >
>>  >>
>>  >>  Focusing on user-space: 1) xnarch_atomic_xchg is meant to work on long types; 2)
>>  >>  set, get routines are not defined in that scope. If the purpose is to define
>>  >>  integer-type ops to handle pointer-type data atomically (i.e. intptr_t), then I
>>  >>  would rather check whether we actually need non-long support at all in
>>  >>  user-space. In case we don't, I would simply reply on the existing
>>  >>  implementation of asm-*/atomic.h + the set / get extensions you provide.
>>  >
>>  > I use both atomic_t and atomic_ptr_t for the implementation of
>>  > user-space mutexes. The problem is that I am constrained by the size
>>  > of pthread_mutex_t, so the "control block read-write locks"
>>  > implementation use atomic_t.
>>  >
>>
>>  Ok, makes sense. Let's just fix the namespace then, so that we don't get the
>>  feeling of having duplicate sets of operations.
> 
> That said, there is just enough room for replacing the atomic_t with
> an atomic_long_t. So, we can make xnarch_atomic_t a long type. Have
> you anything agains making an xnarch_atomic_ptr_t ?
> 

No objection, just let us call it xnarch_atomic_intptr_t to ANSIfy this a bit
more, and clearly state that we need this type to hold a pointer into an integer
value, and operate atomically on it.

-- 
Philippe.

Re: [Xenomai-core] [3/9] Define more atomic operations in user-space

[Gilles Chanteperdrix]

Philippe Gerum wrote:
 > Gilles Chanteperdrix wrote:
 > > On Fri, Apr 25, 2008 at 4:01 PM, Philippe Gerum <rpm@xenomai.org> wrote:
 > >> Gilles Chanteperdrix wrote:
 > >>  > On Fri, Apr 25, 2008 at 3:42 PM, Philippe Gerum <rpm@xenomai.org> wrote:
 > >>  >> Gilles Chanteperdrix wrote:
 > >>  >>  > On Fri, Apr 25, 2008 at 9:48 AM, Philippe Gerum <rpm@xenomai.org> wrote:
 > >>  >>  >> Gilles Chanteperdrix wrote:
 > >>  >>  >>  > This patch implements the _read, _set, and _cmpxchg operations on atomic_long_t
 > >>  >>  >>  > and atomic_ptr_t in user-space in include/asm-generic/atomic.h which should be
 > >>  >>  >>  > included at the end of include/asm-*/atomic.h after the definition of the same
 > >>  >>  >>  > operations for the atomic_t type and atomic64_t type on 64 bits platforms.
 > >>  >>  >>  >
 > >>  >>  >>  > These operations are the basic operations used by user-space mutexes. Maybe we
 > >>  >>  >>  > should add the xnarch_ prefix ?
 > >>  >>  >>  >
 > >>  >>  >>
 > >>  >>  >>  Yes, but more generally, we should rework this to fit the existing atomic
 > >>  >>  >>  support in include/asm-*/atomic.h, so that we don't end up with sideways to what
 > >>  >>  >>  has been already designed to support set, get, xchg and the like, in both kernel
 > >>  >>  >>  and userland context.
 > >>  >>  >
 > >>  >>  > That is not exactly sideways... Linux include/asm-generic/atomic.h
 > >>  >>  > defines operations for atomic_long_t. This file adds two things:
 > >>  >>  > - support for atomic_long_t in user-space (where we can not include
 > >>  >>  > linux include/asm-generic/atomic.h)
 > >>  >>  > - support for a new type atomic_ptr_t both to kernel-space and
 > >>  >>  > user-space, the aim is to avoid all the casts that would take place if
 > >>  >>  > we wanted to use atomic_long_t to store pointers.
 > >>  >>  >
 > >>  >>  > However for this file to work, it has to be included by asm-*/atomic.h
 > >>  >>  > after the definition of atomic_t (and atomic64_t on 64 bits
 > >>  >>  > platforms). So linux includes asm-generic/atomic.h at the end of
 > >>  >>  > asm/atomic.h, I simply reproduced this scheme with Xenomai
 > >>  >>  > include/asm-*/atomic.h.
 > >>  >>  >
 > >>  >>
 > >>  >>  Focusing on user-space: 1) xnarch_atomic_xchg is meant to work on long types; 2)
 > >>  >>  set, get routines are not defined in that scope. If the purpose is to define
 > >>  >>  integer-type ops to handle pointer-type data atomically (i.e. intptr_t), then I
 > >>  >>  would rather check whether we actually need non-long support at all in
 > >>  >>  user-space. In case we don't, I would simply reply on the existing
 > >>  >>  implementation of asm-*/atomic.h + the set / get extensions you provide.
 > >>  >
 > >>  > I use both atomic_t and atomic_ptr_t for the implementation of
 > >>  > user-space mutexes. The problem is that I am constrained by the size
 > >>  > of pthread_mutex_t, so the "control block read-write locks"
 > >>  > implementation use atomic_t.
 > >>  >
 > >>
 > >>  Ok, makes sense. Let's just fix the namespace then, so that we don't get the
 > >>  feeling of having duplicate sets of operations.
 > > 
 > > That said, there is just enough room for replacing the atomic_t with
 > > an atomic_long_t. So, we can make xnarch_atomic_t a long type. Have
 > > you anything agains making an xnarch_atomic_ptr_t ?
 > > 
 > 
 > No objection, just let us call it xnarch_atomic_intptr_t to ANSIfy this a bit
 > more, and clearly state that we need this type to hold a pointer into an integer
 > value, and operate atomically on it.

Ok. The current type defined in asm-arm/atomic.h is
atomic_counter_t. Should I rename this xnarch_atomic_t ?

I ask this now, because I am going to do search and replaces of atomic_
all over my changes, so, I prefer to know before.

-- 


					    Gilles.

Re: [Xenomai-core] [3/9] Define more atomic operations in user-space

[Philippe Gerum]

Gilles Chanteperdrix wrote:
> Philippe Gerum wrote:
>  > Gilles Chanteperdrix wrote:
>  > > On Fri, Apr 25, 2008 at 4:01 PM, Philippe Gerum <rpm@xenomai.org> wrote:
>  > >> Gilles Chanteperdrix wrote:
>  > >>  > On Fri, Apr 25, 2008 at 3:42 PM, Philippe Gerum <rpm@xenomai.org> wrote:
>  > >>  >> Gilles Chanteperdrix wrote:
>  > >>  >>  > On Fri, Apr 25, 2008 at 9:48 AM, Philippe Gerum <rpm@xenomai.org> wrote:
>  > >>  >>  >> Gilles Chanteperdrix wrote:
>  > >>  >>  >>  > This patch implements the _read, _set, and _cmpxchg operations on atomic_long_t
>  > >>  >>  >>  > and atomic_ptr_t in user-space in include/asm-generic/atomic.h which should be
>  > >>  >>  >>  > included at the end of include/asm-*/atomic.h after the definition of the same
>  > >>  >>  >>  > operations for the atomic_t type and atomic64_t type on 64 bits platforms.
>  > >>  >>  >>  >
>  > >>  >>  >>  > These operations are the basic operations used by user-space mutexes. Maybe we
>  > >>  >>  >>  > should add the xnarch_ prefix ?
>  > >>  >>  >>  >
>  > >>  >>  >>
>  > >>  >>  >>  Yes, but more generally, we should rework this to fit the existing atomic
>  > >>  >>  >>  support in include/asm-*/atomic.h, so that we don't end up with sideways to what
>  > >>  >>  >>  has been already designed to support set, get, xchg and the like, in both kernel
>  > >>  >>  >>  and userland context.
>  > >>  >>  >
>  > >>  >>  > That is not exactly sideways... Linux include/asm-generic/atomic.h
>  > >>  >>  > defines operations for atomic_long_t. This file adds two things:
>  > >>  >>  > - support for atomic_long_t in user-space (where we can not include
>  > >>  >>  > linux include/asm-generic/atomic.h)
>  > >>  >>  > - support for a new type atomic_ptr_t both to kernel-space and
>  > >>  >>  > user-space, the aim is to avoid all the casts that would take place if
>  > >>  >>  > we wanted to use atomic_long_t to store pointers.
>  > >>  >>  >
>  > >>  >>  > However for this file to work, it has to be included by asm-*/atomic.h
>  > >>  >>  > after the definition of atomic_t (and atomic64_t on 64 bits
>  > >>  >>  > platforms). So linux includes asm-generic/atomic.h at the end of
>  > >>  >>  > asm/atomic.h, I simply reproduced this scheme with Xenomai
>  > >>  >>  > include/asm-*/atomic.h.
>  > >>  >>  >
>  > >>  >>
>  > >>  >>  Focusing on user-space: 1) xnarch_atomic_xchg is meant to work on long types; 2)
>  > >>  >>  set, get routines are not defined in that scope. If the purpose is to define
>  > >>  >>  integer-type ops to handle pointer-type data atomically (i.e. intptr_t), then I
>  > >>  >>  would rather check whether we actually need non-long support at all in
>  > >>  >>  user-space. In case we don't, I would simply reply on the existing
>  > >>  >>  implementation of asm-*/atomic.h + the set / get extensions you provide.
>  > >>  >
>  > >>  > I use both atomic_t and atomic_ptr_t for the implementation of
>  > >>  > user-space mutexes. The problem is that I am constrained by the size
>  > >>  > of pthread_mutex_t, so the "control block read-write locks"
>  > >>  > implementation use atomic_t.
>  > >>  >
>  > >>
>  > >>  Ok, makes sense. Let's just fix the namespace then, so that we don't get the
>  > >>  feeling of having duplicate sets of operations.
>  > > 
>  > > That said, there is just enough room for replacing the atomic_t with
>  > > an atomic_long_t. So, we can make xnarch_atomic_t a long type. Have
>  > > you anything agains making an xnarch_atomic_ptr_t ?
>  > > 
>  > 
>  > No objection, just let us call it xnarch_atomic_intptr_t to ANSIfy this a bit
>  > more, and clearly state that we need this type to hold a pointer into an integer
>  > value, and operate atomically on it.
> 
> Ok. The current type defined in asm-arm/atomic.h is
> atomic_counter_t. Should I rename this xnarch_atomic_t ?
>

Fine with me.

> I ask this now, because I am going to do search and replaces of atomic_
> all over my changes, so, I prefer to know before.
> 


-- 
Philippe.