All of lore.kernel.org
 help / color / mirror / Atom feed
* [Xenomai-core] [Patch 0/7] Posix skin user-space mutexes, second take.
@ 2008-05-02 22:27 Gilles Chanteperdrix
  2008-05-02 22:30 ` [Xenomai-core] [Patch 1/7] Support for non cached memory mappings Gilles Chanteperdrix
  2008-05-03 19:18 ` [Xenomai-core] [Patch 0/7] Posix skin user-space mutexes, second take Gilles Chanteperdrix
  0 siblings, 2 replies; 34+ messages in thread
From: Gilles Chanteperdrix @ 2008-05-02 22:27 UTC (permalink / raw)
  To: xenomai


Hi,

here comes a second attempt of implementing user-space mutexes for the posix
skin.

Only differences with the first implementation are explained in the following
mails.

Thanks in advance for your review.

-- 


					    Gilles.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Xenomai-core] [Patch 1/7] Support for non cached memory mappings
  2008-05-02 22:27 [Xenomai-core] [Patch 0/7] Posix skin user-space mutexes, second take Gilles Chanteperdrix
@ 2008-05-02 22:30 ` Gilles Chanteperdrix
  2008-05-02 22:32   ` [Xenomai-core] [Patch 2/7] Define XNARCH_SHARED_HEAP_FLAGS Gilles Chanteperdrix
  2008-05-18 16:30   ` [Xenomai-core] [Patch 1/7] Support for non cached memory mappings Philippe Gerum
  2008-05-03 19:18 ` [Xenomai-core] [Patch 0/7] Posix skin user-space mutexes, second take Gilles Chanteperdrix
  1 sibling, 2 replies; 34+ messages in thread
From: Gilles Chanteperdrix @ 2008-05-02 22:30 UTC (permalink / raw)
  To: xenomai


In addition to support for non cached memory mappings, this patch implements
xnheap_init_mapped and xnheap_destroy_mapped in the !CONFIG_XENO_OPT_PERVASIVE
case. This avoids a lot of #ifdefs for users of these functions without
user-space support (posix skin shared memories, and the new semaphore heaps,
heaps where is allocated the memory used for storing semaphores counters and
mutexes owner fields).

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

Index: include/native/heap.h
===================================================================
--- include/native/heap.h	(revision 3738)
+++ 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 3738)
+++ 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 3738)
+++ 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 3738)
+++ 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 3738)
+++ 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)
@@ -1284,11 +1297,39 @@ int xnheap_destroy_mapped(xnheap_t *heap
 	return 0;
 }
 
-EXPORT_SYMBOL(xnheap_init_mapped);
-EXPORT_SYMBOL(xnheap_destroy_mapped);
+#else /* !CONFIG_XENO_OPT_PERVASIVE */
+static void xnheap_free_extent(xnheap_t *heap,
+			       void *extent, u_long size, void *cookie)
+{
+	xnarch_free_host_mem(extent, size);
+}
+
+int xnheap_init_mapped(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 xnheap_destroy_mapped(xnheap_t *heap)
+{
+	xnheap_destroy(heap, &xnheap_free_extent, NULL);
+}
 #endif /* CONFIG_XENO_OPT_PERVASIVE */
 
+EXPORT_SYMBOL(xnheap_init_mapped);
+EXPORT_SYMBOL(xnheap_destroy_mapped);
+
 /*@}*/
 
 EXPORT_SYMBOL(xnheap_alloc);

-- 


					    Gilles.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Xenomai-core] [Patch 2/7] Define XNARCH_SHARED_HEAP_FLAGS
  2008-05-02 22:30 ` [Xenomai-core] [Patch 1/7] Support for non cached memory mappings Gilles Chanteperdrix
@ 2008-05-02 22:32   ` Gilles Chanteperdrix
  2008-05-02 22:33     ` [Xenomai-core] [Patch 3/7] Define more atomic operations in user-space Gilles Chanteperdrix
  2008-05-18 16:30   ` [Xenomai-core] [Patch 1/7] Support for non cached memory mappings Philippe Gerum
  1 sibling, 1 reply; 34+ messages in thread
From: Gilles Chanteperdrix @ 2008-05-02 22:32 UTC (permalink / raw)
  To: xenomai


No comment.

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

Index: include/asm-generic/system.h
===================================================================
--- include/asm-generic/system.h	(revision 3738)
+++ 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 3738)
+++ include/asm-arm/hal.h	(working copy)
@@ -108,10 +108,15 @@ 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
+#ifdef CONFIG_XENO_OPT_PERVASIVE
+#define RTHAL_SHARED_HEAP_FLAGS (cache_is_vivt() ? XNHEAP_GFP_NONCACHED : 0)
+#else /* !CONFIG_XENO_OPT_PERVASIVE */
+#define RTHAL_SHARED_HEAP_FLAGS 0
+#endif /* !CONFIG_XENO_OPT_PERVASIVE */
 
 #define rthal_grab_control()     do { } while(0)
 #define rthal_release_control()  do { } while(0)

-- 


					    Gilles.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Xenomai-core] [Patch 3/7] Define more atomic operations in user-space
  2008-05-02 22:32   ` [Xenomai-core] [Patch 2/7] Define XNARCH_SHARED_HEAP_FLAGS Gilles Chanteperdrix
@ 2008-05-02 22:33     ` Gilles Chanteperdrix
  2008-05-02 22:34       ` [Xenomai-core] [Patch 4/7] Define ARM " Gilles Chanteperdrix
  0 siblings, 1 reply; 34+ messages in thread
From: Gilles Chanteperdrix @ 2008-05-02 22:33 UTC (permalink / raw)
  To: xenomai


This patch now only implement the _get, _set and _cmpxchg operations for the new
xnarch_atomic_intptr_t type. Since xnarch_atomic_t is now a long type, the
atomic_long_t is no longer needed.

---
 Makefile.am |    2 +-
 atomic.h    |   24 ++++++++++++++++++++++++
 2 files changed, 25 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,24 @@
+#ifndef _XENO_ASM_GENERIC_ATOMIC_H
+#define _XENO_ASM_GENERIC_ATOMIC_H
+
+typedef xnarch_atomic_t xnarch_atomic_intptr_t;
+
+static inline void *xnarch_atomic_intptr_get(xnarch_atomic_intptr_t *l)
+{
+        xnarch_atomic_t *v = (xnarch_atomic_t *)l;
+
+        return (void *)xnarch_atomic_get(v);
+}
+
+static inline void xnarch_atomic_intptr_set(xnarch_atomic_intptr_t *l, void *i)
+{
+        xnarch_atomic_t *v = (xnarch_atomic_t *)l;
+
+        xnarch_atomic_set(v, (long)i);
+}
+
+#define xnarch_atomic_intptr_cmpxchg(l, old, new) \
+        (void *)(xnarch_atomic_cmpxchg((xnarch_atomic_t *)(l), \
+				       (long)(old), (long)(new)))
+
+#endif /* _XENO_ASM_GENERIC_ATOMIC_H */
Index: include/asm-generic/Makefile.am
===================================================================
--- include/asm-generic/Makefile.am	(revision 3738)
+++ 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 atomic.h features.h hal.h syscall.h system.h wrappers.h
 
 SUBDIRS = bits

-- 


					    Gilles.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Xenomai-core] [Patch 4/7] Define ARM atomic operations in user-space
  2008-05-02 22:33     ` [Xenomai-core] [Patch 3/7] Define more atomic operations in user-space Gilles Chanteperdrix
@ 2008-05-02 22:34       ` Gilles Chanteperdrix
  2008-05-02 22:35         ` [Xenomai-core] [Patch 5/7] Define new syscalls for the system skin Gilles Chanteperdrix
  2008-05-05 16:33         ` [Xenomai-core] [Patch 4/7] Define ARM atomic operations in user-space Gilles Chanteperdrix
  0 siblings, 2 replies; 34+ messages in thread
From: Gilles Chanteperdrix @ 2008-05-02 22:34 UTC (permalink / raw)
  To: xenomai


The include/asm-arm/atomic.h header now defines the xnarch_memory_barrier in
addition to user-space atomic operations. The pxa3xx deserves a special
treatment since it uses the ARMv6 memory barrier operation whereas being an
ARMv5 for other operations, hence a special --enable-arm-mach=pxa3xx option in
configure.in.

---
 configure.in             |   13 ++
 include/asm-arm/atomic.h |  238 +++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 222 insertions(+), 29 deletions(-)

Index: configure.in
===================================================================
--- configure.in	(revision 3738)
+++ configure.in	(working copy)
@@ -166,6 +166,7 @@ if test $XENO_TARGET_ARCH = arm ; then
   CONFIX_XENO_ARM_EABI=
 
   unset CONFIG_XENO_ARM_SA1100
+  unset CONFIG_XENO_CPU_XSC3
   unset tsc_type
   unset CONFIG_XENO_ARM_HW_DIRECT_TSC
 
@@ -173,7 +174,7 @@ if test $XENO_TARGET_ARCH = arm ; then
   AC_ARG_ENABLE(arm-mach,
 	AS_HELP_STRING([--enable-arm-mach], [Select for which machine we are
 compiling. Valid machines are at91rm9200, at91sam926x, imx, imx21, integrator, 
-ixp4xx, pxa, s3c2410, sa1100.]),
+ixp4xx, pxa, pxa3xx, s3c2410, sa1100.]),
 	[case "$enableval" in
 	at91rm9200)   	arch=4
 	        	tsc_type=__XN_TSC_TYPE_FREERUNNING;;
@@ -193,6 +194,9 @@ ixp4xx, pxa, s3c2410, sa1100.]),
 	ixp4xx|pxa) 	arch=5
 	      		tsc_type=__XN_TSC_TYPE_FREERUNNING;;
 
+	pxa3xx)	        arch=xscale3
+			tsc_type=__XN_TSC_TYPE_FREERUNNING;;
+
 	s3c2410) 	arch=4
 			tsc_type=__XN_TSC_TYPE_DECREMENTER;;
 
@@ -218,6 +222,9 @@ ixp4xx, pxa, s3c2410, sa1100.]),
 		esac
 		CONFIG_XENO_ARM_ARCH="$enableval"
     	])
+  elif test $arch = xscale3; then
+	CONFIG_XENO_ARM_ARCH=5
+	CONFIG_XENO_CPU_XSC3=y
   else
 	CONFIG_XENO_ARM_ARCH=$arch
   fi
@@ -226,6 +233,9 @@ ixp4xx, pxa, s3c2410, sa1100.]),
   AC_MSG_CHECKING(for ARM SA1100 architecture)
   AC_MSG_RESULT(${CONFIG_XENO_ARM_SA1100:-no})
 
+  AC_MSG_CHECKING(for ARM Xscale3 architecture)
+  AC_MSG_RESULT(${CONFIG_XENO_CPU_XSC3:-no})
+
   AC_MSG_CHECKING(for TSC emulation in user-space.)
   if test $tsc_type; then
 	AC_ARG_ENABLE(arm-tsc,
@@ -527,6 +537,7 @@ test x$CONFIG_X86_TSC = xy && AC_DEFINE(
 test -n "$CONFIG_XENO_ARM_ARCH" && AC_DEFINE_UNQUOTED(CONFIG_XENO_ARM_ARCH,$CONFIG_XENO_ARM_ARCH,[config])
 
 test x$CONFIG_XENO_ARM_SA1100 = xy && AC_DEFINE(CONFIG_XENO_ARM_SA1100,1,[config])
+test x$CONFIG_XENO_CPU_XSC3 = xy && AC_DEFINE(CONFIG_XENO_CPU_XSC3,1,[config])
 test $CONFIG_XENO_ARM_HW_DIRECT_TSC && AC_DEFINE_UNQUOTED(CONFIG_XENO_ARM_HW_DIRECT_TSC,$CONFIG_XENO_ARM_HW_DIRECT_TSC,[config])
 test x$CONFIG_XENO_ARM_EABI = xy && AC_DEFINE(CONFIG_XENO_ARM_EABI,1,[config])
 
Index: include/asm-arm/atomic.h
===================================================================
--- include/asm-arm/atomic.h	(revision 3738)
+++ 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>
@@ -31,11 +30,17 @@
 #include <asm/system.h>
 #include <asm/xenomai/features.h>
 
+typedef atomic_t atomic_counter_t;
+typedef atomic_t xnarch_atomic_t;
+
 #define xnarch_atomic_xchg(ptr,v)       xchg(ptr,v)
 #define xnarch_memory_barrier()  	smp_mb()
 
 #if __LINUX_ARM_ARCH__ >= 6
-static inline void atomic_set_mask(unsigned long mask, unsigned long *addr)
+#define XNARCH_HAVE_US_ATOMIC_CMPXCHG
+
+static inline void
+xnarch_atomic_set_mask(unsigned long *addr, unsigned long mask)
 {
     unsigned long tmp, tmp2;
 
@@ -50,7 +55,8 @@ static inline void atomic_set_mask(unsig
     : "cc");
 }
 #else /* ARM_ARCH_6 */
-static inline void atomic_set_mask(unsigned long mask, unsigned long *addr)
+static inline void
+xnarch_atomic_set_mask(unsigned long *addr, unsigned long mask)
 {
     unsigned long flags;
 
@@ -58,25 +64,32 @@ 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)
+#define xnarch_atomic_set(pcounter,i)          atomic_set((pcounter),i)
 #define xnarch_atomic_get(pcounter)            atomic_read(pcounter)
 #define xnarch_atomic_inc(pcounter)            atomic_inc(pcounter)
 #define xnarch_atomic_dec(pcounter)            atomic_dec(pcounter)
+#define xnarch_atomic_clear_mask(addr, mask)   atomic_clear_mask((mask), (addr))
+#define xnarch_atomic_cmpxchg(pcounter, old, new) \
+	atomic_cmpxchg((pcounter), (old), (new))
 #define xnarch_atomic_inc_and_test(pcounter)   atomic_inc_and_test(pcounter)
 #define xnarch_atomic_dec_and_test(pcounter)   atomic_dec_and_test(pcounter)
-#define xnarch_atomic_set_mask(pflags,mask)    atomic_set_mask(mask,pflags)
-#define xnarch_atomic_clear_mask(pflags,mask)  atomic_clear_mask(mask,pflags)
-
-typedef atomic_t atomic_counter_t;
 
 #else /* !__KERNEL__ */
 
 #include <asm/xenomai/features.h>
 #include <asm/xenomai/syscall.h>
+#include <nucleus/compiler.h>
+
+typedef struct { volatile int counter; } xnarch_atomic_t;
 
-typedef struct { volatile int counter; } atomic_counter_t;
+#define xnarch_atomic_get(v)	((v)->counter)
 
 /*
  * This function doesn't exist, so you'll get a linker error
@@ -129,12 +142,47 @@ __xchg(volatile void *ptr, unsigned long
  * Atomic operations lifted from linux/include/asm-arm/atomic.h 
  */
 #if CONFIG_XENO_ARM_ARCH >= 6
-static __inline__ int atomic_add_return(int i, atomic_counter_t *v)
+#define XNARCH_HAVE_US_ATOMIC_CMPXCHG
+
+static __inline__ void xnarch_atomic_set(xnarch_atomic_t *v, int i)
+{
+	unsigned long tmp;
+
+	__asm__ __volatile__("@ xnarch_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
+xnarch_atomic_cmpxchg(xnarch_atomic_t *ptr, int old, int new)
+{
+	unsigned long oldval, res;
+
+	do {
+		__asm__ __volatile__("@ xnarch_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 xnarch_atomic_add_return(int i, xnarch_atomic_t *v)
 {
     unsigned long tmp;
     int result;
 
-    __asm__ __volatile__("@ atomic_add_return\n"
+    __asm__ __volatile__("@ xnarch_atomic_add_return\n"
 "1: ldrex   %0, [%2]\n"
 "   add     %0, %0, %3\n"
 "   strex   %1, %0, [%2]\n"
@@ -147,12 +195,12 @@ static __inline__ int atomic_add_return(
     return result;
 }
 
-static __inline__ int atomic_sub_return(int i, atomic_counter_t *v)
+static __inline__ int xnarch_atomic_sub_return(int i, xnarch_atomic_t *v)
 {
     unsigned long tmp;
     int result;
 
-    __asm__ __volatile__("@ atomic_sub_return\n"
+    __asm__ __volatile__("@ xnarch_atomic_sub_return\n"
 "1: ldrex   %0, [%2]\n"
 "   sub     %0, %0, %3\n"
 "   strex   %1, %0, [%2]\n"
@@ -165,11 +213,12 @@ static __inline__ int atomic_sub_return(
     return result;
 }
 
-static __inline__ void atomic_set_mask(unsigned long mask, unsigned long *addr)
+static __inline__ void
+xnarch_atomic_set_mask(unsigned long *addr, unsigned long mask)
 {
     unsigned long tmp, tmp2;
 
-    __asm__ __volatile__("@ atomic_set_mask\n"
+    __asm__ __volatile__("@ xnarch_atomic_set_mask\n"
 "1: ldrex   %0, [%2]\n"
 "   orr     %0, %0, %3\n"
 "   strex   %1, %0, [%2]\n"
@@ -180,11 +229,12 @@ static __inline__ void atomic_set_mask(u
     : "cc");
 }
 
-static __inline__ void atomic_clear_mask(unsigned long mask, unsigned long *addr)
+static __inline__ void
+xnarch_atomic_clear_mask(unsigned long *addr, unsigned long mask)
 {
     unsigned long tmp, tmp2;
 
-    __asm__ __volatile__("@ atomic_clear_mask\n"
+    __asm__ __volatile__("@ xnarch_atomic_clear_mask\n"
 "1: ldrex   %0, [%2]\n"
 "   bic     %0, %0, %3\n"
 "   strex   %1, %0, [%2]\n"
@@ -194,8 +244,9 @@ static __inline__ void atomic_clear_mask
     : "r" (addr), "Ir" (mask)
     : "cc");
 }
-#else /* ARM_ARCH_6 */
-static __inline__ int atomic_add_return(int i, atomic_counter_t *v)
+
+#elif CONFIG_SMP
+static __inline__ int xnarch_atomic_add_return(int i, xnarch_atomic_t *v)
 {
     int ret;
 
@@ -204,7 +255,7 @@ static __inline__ int atomic_add_return(
     return ret;
 }
 
-static __inline__ int atomic_sub_return(int i, atomic_counter_t *v)
+static __inline__ int xnarch_atomic_sub_return(int i, xnarch_atomic_t *v)
 {
     int ret;
 
@@ -213,25 +264,153 @@ static __inline__ int atomic_sub_return(
     return ret;
 }
 
-static inline void atomic_set_mask(unsigned long mask, unsigned long *addr)
+static inline void
+xnarch_atomic_set_mask(unsigned long *addr, unsigned long mask)
 {
     XENOMAI_SYSCALL3(__xn_sys_arch,
                      XENOMAI_SYSARCH_ATOMIC_SET_MASK, mask, addr);
 }
 
-static inline void atomic_clear_mask(unsigned long mask, unsigned long *addr)
+static inline void
+xnarch_atomic_clear_mask(unsigned long *addr, unsigned long mask)
 {
     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 xnarch_atomic_set(xnarch_atomic_t *ptr, int val)
+{
+	ptr->counter = val;
+}
 
-#define xnarch_memory_barrier()                 __asm__ __volatile__("": : :"memory")
+static __inline__ int
+xnarch_atomic_cmpxchg(xnarch_atomic_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 xnarch_atomic_add_return(int i, xnarch_atomic_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: @ xnarch_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 xnarch_atomic_sub_return(int i, xnarch_atomic_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: @ xnarch_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 xnarch_atomic_set_mask(xnarch_atomic_t *v, long mask)
+{
+	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: @ xnarch_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 xnarch_atomic_clear_mask(xnarch_atomic_t *v, long mask)
+{
+	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: @ xnarch_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 */
+
+#if CONFIG_XENO_ARM_ARCH >= 7
+#define xnarch_memory_barrier() \
+	__asm__ __volatile__ ("dmb" : : : "memory")
+#elif defined(CONFIG_XENO_CPU_XSC3) || CONFIG_XENO_ARM_ARCH == 6
+#define xnarch_memory_barrier() \
+	__asm__ __volatile__ ("mcr p15, 0, %0, c7, c10, 5" \
+                              : : "r" (0) : "memory")
+#else /* CONFIG_XENO_ARM_ARCH <= 5 */
+#define xnarch_memory_barrier() \
+	__asm__ __volatile__ ("": : :"memory")
+#endif /* CONFIG_XENO_ARM_ARCH <= 5 */
 
-#define xnarch_atomic_inc(pcounter)             (void) atomic_add_return(1, pcounter)
-#define xnarch_atomic_dec_and_test(pcounter)    (atomic_sub_return(1, pcounter) == 0)
-#define xnarch_atomic_set_mask(pflags,mask)     atomic_set_mask(mask,pflags)
-#define xnarch_atomic_clear_mask(pflags,mask)   atomic_clear_mask(mask,pflags)
+#define xnarch_atomic_inc(pcounter)             (void) xnarch_atomic_add_return(1, pcounter)
+#define xnarch_atomic_dec_and_test(pcounter)    (xnarch_atomic_sub_return(1, pcounter) == 0)
 
 #define cpu_relax()                             xnarch_memory_barrier()
 #define xnarch_read_memory_barrier()		xnarch_memory_barrier()
@@ -241,6 +420,9 @@ static inline void atomic_clear_mask(uns
 
 typedef unsigned long atomic_flags_t;
 
+/* Add support for xnarch_atomic_intptr_t */
+#include <asm-generic/xenomai/atomic.h>
+
 #endif /* !_XENO_ASM_ARM_ATOMIC_H */
 
 // vim: ts=4 et sw=4 sts=4


-- 


					    Gilles.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Xenomai-core] [Patch 5/7] Define new syscalls for the system skin
  2008-05-02 22:34       ` [Xenomai-core] [Patch 4/7] Define ARM " Gilles Chanteperdrix
@ 2008-05-02 22:35         ` Gilles Chanteperdrix
  2008-05-02 22:36           ` [Xenomai-core] [Patch 6/7] Re-implementation of mutexes, kernel-space support Gilles Chanteperdrix
  2008-05-18 16:40           ` [Xenomai-core] [Patch 5/7] Define new syscalls for the system skin Philippe Gerum
  2008-05-05 16:33         ` [Xenomai-core] [Patch 4/7] Define ARM atomic operations in user-space Gilles Chanteperdrix
  1 sibling, 2 replies; 34+ messages in thread
From: Gilles Chanteperdrix @ 2008-05-02 22:35 UTC (permalink / raw)
  To: xenomai


The two syscalls defined in the posix skin now moved to the sys skin, they are
used in user-space by include/asm-generic/bits/bind.h and the new header
include/asm-generic/bits/current.h. The global and process-specific shared heaps
are now part of this patch.

---
 include/asm-generic/bits/Makefile.am |    1
 include/asm-generic/bits/bind.h      |  121 +++++++++++++++++++++++++++++++++++
 include/asm-generic/bits/current.h   |   14 ++++
 include/asm-generic/syscall.h        |    2
 include/nucleus/Makefile.am          |    1
 include/nucleus/sys_ppd.h            |   37 ++++++++++
 include/posix/syscall.h              |    1
 ksrc/nucleus/Kconfig                 |   25 +++++++
 ksrc/nucleus/module.c                |   19 ++++-
 ksrc/nucleus/shadow.c                |  103 ++++++++++++++++++++++++++++-
 src/skins/posix/thread.c             |    4 +
 11 files changed, 323 insertions(+), 5 deletions(-)

Index: include/asm-generic/syscall.h
===================================================================
--- include/asm-generic/syscall.h	(revision 3738)
+++ include/asm-generic/syscall.h	(working copy)
@@ -30,6 +30,8 @@
 #define __xn_sys_info       4	/* xnshadow_get_info(muxid,&info) */
 #define __xn_sys_arch       5	/* r = xnarch_local_syscall(args) */
 #define __xn_sys_trace      6	/* r = xntrace_xxx(...) */
+#define __xn_sys_sem_heap   7
+#define __xn_sys_current    8
 
 #define XENOMAI_LINUX_DOMAIN  0
 #define XENOMAI_XENO_DOMAIN   1
Index: ksrc/nucleus/Kconfig
===================================================================
--- ksrc/nucleus/Kconfig	(revision 3738)
+++ ksrc/nucleus/Kconfig	(working copy)
@@ -158,6 +158,31 @@ config XENO_OPT_SYS_STACKPOOLSZ
 	default 0
 endif	
 
+config XENO_OPT_SEM_HEAPSZ
+	int "Size of private semaphores heap (Kb)"
+	default 12
+	help
+
+	Xenomai implementation of user-space semaphores relies on heaps 
+	shared between kernel and user-space. This configuration entry
+	allow to set the size of the heap used for private semaphores.
+
+	Note that each semaphore will allocate 4 or 8 bytes of memory,
+	so, the default of 12 Kb allows creating many semaphores.
+
+config XENO_OPT_GLOBAL_SEM_HEAPSZ
+	int "Size of global semaphores heap (Kb)"
+	default 12
+	help
+
+	Xenomai implementation of user-space semaphores relies on heaps 
+	shared between kernel and user-space. This configuration entry
+	allow to set the size of the heap used for semaphores shared 
+	between several processes.
+
+	Note that each semaphore will allocate 4 or 8 bytes of memory,
+	so, the default of 12 Kb allows creating many semaphores.
+
 config XENO_OPT_STATS
 	bool "Statistics collection"
 	depends on PROC_FS
Index: ksrc/nucleus/module.c
===================================================================
--- ksrc/nucleus/module.c	(revision 3738)
+++ ksrc/nucleus/module.c	(working copy)
@@ -28,6 +28,7 @@
 #include <nucleus/timer.h>
 #include <nucleus/heap.h>
 #include <nucleus/version.h>
+#include <nucleus/sys_ppd.h>
 #ifdef CONFIG_XENO_OPT_PIPE
 #include <nucleus/pipe.h>
 #endif /* CONFIG_XENO_OPT_PIPE */
@@ -50,6 +51,8 @@ u_long xnmod_sysheap_size;
 
 int xeno_nucleus_status = -EINVAL;
 
+struct xnsys_ppd __xnsys_global_ppd;
+
 void xnmod_alloc_glinks(xnqueue_t *freehq)
 {
 	xngholder_t *sholder, *eholder;
@@ -1156,6 +1159,14 @@ int __init __xeno_sys_init(void)
 	if (err)
 		goto fail;
 
+	err = xnheap_init_mapped(&__xnsys_global_ppd.sem_heap,
+				 CONFIG_XENO_OPT_GLOBAL_SEM_HEAPSZ * 1024,
+				 XNARCH_SHARED_HEAP_FLAGS ?:
+				 (CONFIG_XENO_OPT_GLOBAL_SEM_HEAPSZ <= 128
+				  ? GFP_USER : 0));
+	if (err)
+		goto cleanup_arch;
+	
 #ifdef __KERNEL__
 #ifdef CONFIG_PROC_FS
 	xnpod_init_proc();
@@ -1167,7 +1178,7 @@ int __init __xeno_sys_init(void)
 	err = xnpipe_mount();
 
 	if (err)
-		goto cleanup_arch;
+		goto cleanup_proc;
 #endif /* CONFIG_XENO_OPT_PIPE */
 
 #ifdef CONFIG_XENO_OPT_PERVASIVE
@@ -1207,7 +1218,7 @@ int __init __xeno_sys_init(void)
 #ifdef CONFIG_XENO_OPT_PIPE
 	xnpipe_umount();
 
-      cleanup_arch:
+      cleanup_proc:
 
 #endif /* CONFIG_XENO_OPT_PIPE */
 
@@ -1215,6 +1226,8 @@ int __init __xeno_sys_init(void)
 	xnpod_delete_proc();
 #endif /* CONFIG_PROC_FS */
 
+      cleanup_arch:
+
 	xnarch_exit();
 
 #endif /* __KERNEL__ */
@@ -1254,6 +1267,8 @@ void __exit __xeno_sys_exit(void)
 #endif /* CONFIG_XENO_OPT_PIPE */
 #endif /* __KERNEL__ */
 
+	xnheap_destroy_mapped(&__xnsys_global_ppd.sem_heap);
+
 	if (nkmsgbuf)
 		xnarch_free_host_mem(nkmsgbuf, XNPOD_FATAL_BUFSZ);
 
Index: ksrc/nucleus/shadow.c
===================================================================
--- ksrc/nucleus/shadow.c	(revision 3738)
+++ ksrc/nucleus/shadow.c	(working copy)
@@ -51,6 +51,7 @@
 #include <nucleus/ppd.h>
 #include <nucleus/trace.h>
 #include <nucleus/stat.h>
+#include <nucleus/sys_ppd.h>
 #include <asm/xenomai/features.h>
 #include <asm/xenomai/syscall.h>
 #include <asm/xenomai/bits/shadow.h>
@@ -1558,11 +1559,11 @@ static void stringify_feature_set(u_long
 
 static int xnshadow_sys_bind(struct pt_regs *regs)
 {
+	xnshadow_ppd_t *ppd = NULL, *sys_ppd = NULL;
 	unsigned magic = __xn_reg_arg1(regs);
 	u_long featdep = __xn_reg_arg2(regs);
 	u_long abirev = __xn_reg_arg3(regs);
 	u_long infarg = __xn_reg_arg4(regs);
-	xnshadow_ppd_t *ppd = NULL;
 	xnfeatinfo_t finfo;
 	u_long featmis;
 	int muxid, err;
@@ -1636,6 +1637,36 @@ static int xnshadow_sys_bind(struct pt_r
 	   earlier than that, do not refer to nkpod until the latter had a
 	   chance to call xnpod_init(). */
 
+	xnlock_get_irqsave(&nklock, s);
+	sys_ppd = ppd_lookup(0, current->mm);
+	xnlock_put_irqrestore(&nklock, s);
+
+	if (sys_ppd)
+		goto muxid_eventcb;
+
+	sys_ppd = (xnshadow_ppd_t *) muxtable[0].props->eventcb(XNSHADOW_CLIENT_ATTACH,
+								 current);
+
+	if (IS_ERR(sys_ppd)) {
+		err = PTR_ERR(sys_ppd);
+		goto fail;
+	}
+
+	if (!sys_ppd)
+		goto muxid_eventcb;
+
+
+	sys_ppd->key.muxid = 0;
+	sys_ppd->key.mm = current->mm;
+
+	if (ppd_insert(sys_ppd) == -EBUSY) {
+		/* In case of concurrent binding (which can not happen with
+		   Xenomai libraries), detach right away the second ppd. */
+		muxtable[0].props->eventcb(XNSHADOW_CLIENT_DETACH, sys_ppd);
+		sys_ppd = NULL;
+	}
+
+  muxid_eventcb:
 	if (!muxtable[muxid].props->eventcb)
 		goto eventcb_done;
 
@@ -1652,7 +1683,7 @@ static int xnshadow_sys_bind(struct pt_r
 
 	if (IS_ERR(ppd)) {
 		err = PTR_ERR(ppd);
-		goto fail;
+		goto fail_destroy_sys_ppd;
 	}
 
 	if (!ppd)
@@ -1689,6 +1720,11 @@ static int xnshadow_sys_bind(struct pt_r
 
 		err = -ENOSYS;
 
+	  fail_destroy_sys_ppd:
+		if (sys_ppd) {
+			ppd_remove(sys_ppd);
+			muxtable[0].props->eventcb(XNSHADOW_CLIENT_DETACH, sys_ppd);
+		}
 	      fail:
 		if (!xnarch_atomic_get(&muxtable[muxid].refcnt))
 			xnarch_atomic_dec(&muxtable[muxid].refcnt);
@@ -1856,6 +1892,32 @@ static int xnshadow_sys_trace(struct pt_
 	return err;
 }
 
+struct heap_info {
+	xnheap_t *addr;
+	unsigned size;
+};
+
+static int xnshadow_sys_sem_heap(struct pt_regs *regs)
+{
+	struct heap_info hinfo, __user *us_hinfo;
+	unsigned global;
+
+	global = __xn_reg_arg2(regs);
+	us_hinfo = (struct heap_info __user *) __xn_reg_arg1(regs);
+	hinfo.addr = &xnsys_ppd_get(global)->sem_heap;
+	hinfo.size = xnheap_extentsize(hinfo.addr);
+
+	return __xn_safe_copy_to_user(us_hinfo, &hinfo, sizeof(*us_hinfo));
+}
+
+static int xnshadow_sys_current(struct pt_regs *regs)
+{
+	xnthread_t * __user *us_current, *cur = xnshadow_thread(current);
+	us_current = (xnthread_t *__user *) __xn_reg_arg1(regs);
+
+	return __xn_safe_copy_to_user(us_current, &cur, sizeof(*us_current));
+}
+
 static xnsysent_t __systab[] = {
 	[__xn_sys_migrate] = {&xnshadow_sys_migrate, __xn_exec_current},
 	[__xn_sys_arch] = {&xnshadow_sys_arch, __xn_exec_any},
@@ -1864,15 +1926,50 @@ static xnsysent_t __systab[] = {
 	[__xn_sys_completion] = {&xnshadow_sys_completion, __xn_exec_lostage},
 	[__xn_sys_barrier] = {&xnshadow_sys_barrier, __xn_exec_lostage},
 	[__xn_sys_trace] = {&xnshadow_sys_trace, __xn_exec_any},
+	[__xn_sys_sem_heap] = {&xnshadow_sys_sem_heap, __xn_exec_any},
+	[__xn_sys_current] = {&xnshadow_sys_current, __xn_exec_any},
 };
 
+static void *xnshadow_sys_event(int event, void *data)
+{
+	struct xnsys_ppd *p;
+	int err;
+
+	switch(event) {
+	case XNSHADOW_CLIENT_ATTACH:
+		p = (struct xnsys_ppd *) xnarch_alloc_host_mem(sizeof(*p));
+		if (!p)
+			return ERR_PTR(-ENOMEM);
+
+		err = xnheap_init_mapped(&p->sem_heap,
+					 CONFIG_XENO_OPT_SEM_HEAPSZ * 1024,
+					 XNARCH_SHARED_HEAP_FLAGS ?:
+					 (CONFIG_XENO_OPT_SEM_HEAPSZ <= 128
+					  ? GFP_USER : 0));
+		if (err) {
+			xnarch_free_host_mem(p, sizeof(*p));
+			return ERR_PTR(err);
+		}
+
+		return &p->ppd;
+
+	case XNSHADOW_CLIENT_DETACH:
+		p = ppd2sys((xnshadow_ppd_t *) data);
+
+		xnheap_destroy_mapped(&p->sem_heap);
+
+		return NULL;
+	}
+
+	return ERR_PTR(-EINVAL);
+}
 
 static struct xnskin_props __props = {
 	.name = "sys",
 	.magic = 0x434F5245,
 	.nrcalls = sizeof(__systab) / sizeof(__systab[0]),
 	.systab = __systab,
-	.eventcb = NULL,
+	.eventcb = xnshadow_sys_event,
 	.timebasep = NULL,
 	.module = NULL
 };
Index: include/posix/syscall.h
===================================================================
--- include/posix/syscall.h	(revision 3738)
+++ 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
Index: src/skins/posix/thread.c
===================================================================
--- src/skins/posix/thread.c	(revision 3738)
+++ src/skins/posix/thread.c	(working copy)
@@ -24,6 +24,7 @@
 #include <sys/types.h>
 #include <semaphore.h>
 #include <posix/syscall.h>
+#include <asm-generic/bits/current.h>
 
 extern int __pse51_muxid;
 
@@ -56,6 +57,7 @@ int __wrap_pthread_setschedparam(pthread
 
 	if (!err && promoted) {
 		old_sigharden_handler = signal(SIGHARDEN, &__pthread_sigharden_handler);
+		xeno_set_current();
 		if (policy != SCHED_OTHER)
 			XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_XENO_DOMAIN);
 	}
@@ -132,6 +134,8 @@ static void *__pthread_trampoline(void *
 	start = iargs->start;
 	cookie = iargs->arg;
 
+	xeno_set_current();
+
 	__real_sem_post(&iargs->sync);
 
 	if (!err) {
Index: include/asm-generic/bits/current.h
===================================================================
--- include/asm-generic/bits/current.h	(revision 0)
+++ include/asm-generic/bits/current.h	(revision 0)
@@ -0,0 +1,14 @@
+#ifndef _XENO_ASM_GENERIC_CURRENT_H
+#define _XENO_ASM_GENERIC_CURRENT_H
+
+#include <pthread.h>
+
+extern pthread_key_t xeno_current_key;
+
+extern void xeno_set_current(void);
+
+static inline void *xeno_get_current(void)
+{
+	return pthread_getspecific(xeno_current_key);
+}
+#endif /* _XENO_ASM_GENERIC_CURRENT_H */
Index: include/asm-generic/bits/Makefile.am
===================================================================
--- include/asm-generic/bits/Makefile.am	(revision 3738)
+++ include/asm-generic/bits/Makefile.am	(working copy)
@@ -2,6 +2,7 @@ includesubdir = $(includedir)/asm-generi
 
 includesub_HEADERS = \
 	bind.h \
+	current.h \
 	heap.h \
 	intr.h \
 	mlock_alert.h \
Index: include/nucleus/sys_ppd.h
===================================================================
--- include/nucleus/sys_ppd.h	(revision 0)
+++ include/nucleus/sys_ppd.h	(revision 0)
@@ -0,0 +1,37 @@
+#ifndef _XENO_NUCLEUS_SYS_PPD_H
+#define _XENO_NUCLEUS_SYS_PPD_H
+
+#include <nucleus/ppd.h>
+#include <nucleus/heap.h>
+
+struct xnsys_ppd {
+	xnshadow_ppd_t ppd;
+	xnheap_t sem_heap;
+
+#define ppd2sys(addr) container_of(addr, struct xnsys_ppd, ppd)
+};
+
+extern struct xnsys_ppd __xnsys_global_ppd;
+
+#ifdef CONFIG_XENO_OPT_PERVASIVE
+
+static inline struct xnsys_ppd *xnsys_ppd_get(int global)
+{
+	xnshadow_ppd_t *ppd;
+
+	if (global || !(ppd = xnshadow_ppd_get(0)))
+		return &__xnsys_global_ppd;
+
+	return ppd2sys(ppd);
+}
+
+#else /* !CONFIG_XENO_OPT_PERVASIVE */
+
+static inline struct xnsys_ppd *xnsys_ppd_get(int global)
+{
+	return &__xnsys_global_ppd;
+}
+
+#endif /* !CONFIG_XENO_OPT_PERVASIVE */
+
+#endif /* _XENO_NUCLEUS_SYS_PPD_H */
Index: include/nucleus/Makefile.am
===================================================================
--- include/nucleus/Makefile.am	(revision 3738)
+++ include/nucleus/Makefile.am	(working copy)
@@ -20,6 +20,7 @@ includesub_HEADERS = \
 	stat.h \
 	synch.h \
 	system.h \
+	sys_ppd.h \
 	thread.h \
 	timebase.h \
 	timer.h \
Index: include/asm-generic/bits/bind.h
===================================================================
--- include/asm-generic/bits/bind.h	(revision 3738)
+++ include/asm-generic/bits/bind.h	(working copy)
@@ -6,10 +6,89 @@
 #include <string.h>
 #include <errno.h>
 #include <signal.h>
+#include <pthread.h>
+#include <sys/ioctl.h>
 #include <asm/xenomai/syscall.h>
 
+__attribute__ ((weak))
+pthread_key_t xeno_current_key;
+__attribute__ ((weak))
+pthread_once_t xeno_init_current_key_once = PTHREAD_ONCE_INIT;
+
+__attribute__ ((weak))
+void xeno_set_current(void)
+{
+	void *kthread_cb;
+	XENOMAI_SYSCALL1(__xn_sys_current, &kthread_cb);
+	pthread_setspecific(xeno_current_key, kthread_cb);
+}
+
+static void init_current_key(void)
+{
+	int err = pthread_key_create(&xeno_current_key, NULL);
+	if (err) {
+		fprintf(stderr, "Xenomai: error creating TSD key: %s\n",
+			strerror(err));
+		exit(1);
+	}
+}
+
+__attribute__ ((weak))
+unsigned long xeno_sem_heap[2] = { 0, 0 };
+
 void xeno_handle_mlock_alert(int sig);
 
+static void *map_sem_heap(unsigned shared)
+{
+	struct heap_info {
+		void *addr;
+		unsigned size;
+	} hinfo;
+	int fd, err;
+
+	fd = open("/dev/rtheap", O_RDWR, 0);
+	if (fd < 0) {
+		fprintf(stderr, "Xenomai: open: %m\n");
+		return MAP_FAILED;
+	}
+
+	err = XENOMAI_SYSCALL2(__xn_sys_sem_heap, &hinfo, shared);
+	if (err < 0) {
+		fprintf(stderr, "Xenomai: sys_sem_heap: %m\n");
+		return MAP_FAILED;
+	}
+
+	err = ioctl(fd, 0, hinfo.addr);
+	if (err < 0) {
+		fprintf(stderr, "Xenomai: ioctl: %m\n");
+		return MAP_FAILED;
+	}
+
+	hinfo.addr = mmap(NULL, hinfo.size,
+			  PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
+	close(fd);
+
+	return hinfo.addr;
+}
+
+static void unmap_sem_heap(unsigned long heap_addr, unsigned shared)
+{
+	struct heap_info {
+		void *addr;
+		unsigned size;
+	} hinfo;
+	int err;
+
+	err = XENOMAI_SYSCALL2(__xn_sys_sem_heap, &hinfo, shared);
+	if (err < 0) {
+		fprintf(stderr, "Xenomai: sys_sem_heap: %m\n");
+		return;
+	}
+
+	munmap((void *) heap_addr, hinfo.size);
+}
+
+
 static inline int
 xeno_bind_skin(unsigned skin_magic, const char *skin, const char *module)
 {
@@ -62,6 +141,27 @@ xeno_bind_skin(unsigned skin_magic, cons
 	sa.sa_flags = 0;
 	sigaction(SIGXCPU, &sa, NULL);
 
+	pthread_once(&xeno_init_current_key_once, &init_current_key);
+
+	/* In case we forked, we need to map the new local semaphore heap */
+	if (xeno_sem_heap[0])
+		unmap_sem_heap(xeno_sem_heap[0], 0);
+	xeno_sem_heap[0] = (unsigned long) map_sem_heap(0);
+	if (xeno_sem_heap[0] == (unsigned long) MAP_FAILED) {
+		perror("Xenomai: mmap(local sem heap)");
+		exit(EXIT_FAILURE);
+	}
+
+	/* Even if we forked the global semaphore heap did not change, no need
+	  to map it anew */
+	if (!xeno_sem_heap[1]) {
+		xeno_sem_heap[1] = (unsigned long) map_sem_heap(1);
+		if (xeno_sem_heap[1] == (unsigned long) MAP_FAILED) {
+			perror("Xenomai: mmap(global sem heap)");
+			exit(EXIT_FAILURE);
+		}
+	}
+
 	return muxid;
 }
 
@@ -105,6 +205,27 @@ xeno_bind_skin_opt(unsigned skin_magic, 
 	xeno_arch_features_check();
 #endif /* xeno_arch_features_check */
 
+	pthread_once(&xeno_init_current_key_once, &init_current_key);
+
+	/* In case we forked, we need to map the new local semaphore heap */
+	if (xeno_sem_heap[0])
+		unmap_sem_heap(xeno_sem_heap[0], 0);
+	xeno_sem_heap[0] = (unsigned long) map_sem_heap(0);
+	if (xeno_sem_heap[0] == (unsigned long) MAP_FAILED) {
+		perror("Xenomai: mmap(local sem heap)");
+		exit(EXIT_FAILURE);
+	}
+
+	/* Even if we forked the global semaphore heap did not change, no need
+	  to map it anew */
+	if (!xeno_sem_heap[1]) {
+		xeno_sem_heap[1] = (unsigned long) map_sem_heap(1);
+		if (xeno_sem_heap[1] == (unsigned long) MAP_FAILED) {
+			perror("Xenomai: mmap(global sem heap)");
+			exit(EXIT_FAILURE);
+		}
+	}
+
 	return muxid;
 }
 

-- 


					    Gilles.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Xenomai-core] [Patch 6/7] Re-implementation of mutexes, kernel-space support.
  2008-05-02 22:35         ` [Xenomai-core] [Patch 5/7] Define new syscalls for the system skin Gilles Chanteperdrix
@ 2008-05-02 22:36           ` Gilles Chanteperdrix
  2008-05-02 22:38             ` [Xenomai-core] [Patch 7/7] Re-implementation of mutexes, user-space support Gilles Chanteperdrix
  2008-05-18 16:40           ` [Xenomai-core] [Patch 5/7] Define new syscalls for the system skin Philippe Gerum
  1 sibling, 1 reply; 34+ messages in thread
From: Gilles Chanteperdrix @ 2008-05-02 22:36 UTC (permalink / raw)
  To: xenomai


No comment.

---
 include/posix/pthread.h    |   56 ++++----
 ksrc/skins/posix/cb_lock.h |   84 ++++++++++++
 ksrc/skins/posix/cond.c    |   41 ++++-
 ksrc/skins/posix/mutex.c   |  308 +++++++++++++++++++++++++++------------------
 ksrc/skins/posix/mutex.h   |  125 ++++++++++++++----
 ksrc/skins/posix/shm.c     |   40 -----
 ksrc/skins/posix/syscall.c |  300 ++++++++++++++++++++++++++++++++++++++++++-
 7 files changed, 725 insertions(+), 229 deletions(-)

Index: ksrc/skins/posix/shm.c
===================================================================
--- ksrc/skins/posix/shm.c	(revision 3738)
+++ 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. */
 
 		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. */
 
 			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 = -xnheap_init_mapped(&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: 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(xnarch_atomic_t *lock)
+{
+	unsigned val = xnarch_atomic_get(lock);
+	while (likely(val != -1)) {
+		unsigned old = xnarch_atomic_cmpxchg(lock, val, val + 1);
+		if (likely(old == val))
+			return 0;
+		val = old;
+	}
+	return -EBUSY;
+}
+
+static __inline__ void __cb_read_unlock(xnarch_atomic_t *lock)
+{
+	unsigned old, val = xnarch_atomic_get(lock);
+	while (likely(val != -1)) {
+		old = xnarch_atomic_cmpxchg(lock, val, val - 1);
+		if (likely(old == val))
+			return;
+		val = old;
+	}
+}
+
+static __inline__ int __cb_try_write_lock(xnarch_atomic_t *lock)
+{
+	unsigned old = xnarch_atomic_cmpxchg(lock, 0, -1);
+	if (unlikely(old))
+		return -EBUSY;
+	return 0;
+}
+
+static __inline__ void __cb_force_write_lock(xnarch_atomic_t *lock)
+{
+	xnarch_atomic_set(lock, -1);
+}
+
+static __inline__ void __cb_write_unlock(xnarch_atomic_t *lock)
+{
+	xnarch_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 */
Index: include/posix/pthread.h
===================================================================
--- include/posix/pthread.h	(revision 3738)
+++ include/posix/pthread.h	(working copy)
@@ -93,19 +93,6 @@ typedef struct pse51_threadattr {
 
 /* pthread_mutexattr_t and pthread_condattr_t fit on 32 bits, for compatibility
    with libc. */
-typedef struct pse51_mutexattr {
-	unsigned magic: 24;
-	unsigned type: 2;
-	unsigned protocol: 2;
-	unsigned pshared: 1;
-} pthread_mutexattr_t;
-
-typedef struct pse51_condattr {
-	unsigned magic: 24;
-	unsigned clock: 2;
-	unsigned pshared: 1;
-} pthread_condattr_t;
-
 struct pse51_key;
 typedef struct pse51_key *pthread_key_t;
 
@@ -169,24 +156,44 @@ struct timespec;
 #define PTHREAD_IENABLE     0
 #define PTHREAD_IDISABLE    1
 
+struct pse51_mutexattr {
+	unsigned magic: 24;
+	unsigned type: 2;
+	unsigned protocol: 2;
+	unsigned pshared: 1;
+};
+
+struct pse51_condattr {
+	unsigned magic: 24;
+	unsigned clock: 2;
+	unsigned pshared: 1;
+};
+
 struct pse51_mutex;
 
 union __xeno_mutex {
-    pthread_mutex_t native_mutex;
-    struct __shadow_mutex {
-	unsigned magic;
-	struct pse51_mutex *mutex;
-    } shadow_mutex;
+	pthread_mutex_t native_mutex;
+	struct __shadow_mutex {
+		unsigned magic;
+		unsigned lockcnt;
+		struct pse51_mutex *mutex;
+		xnarch_atomic_t lock;
+		union {
+			unsigned owner_offset;
+			xnarch_atomic_intptr_t *owner;
+		};
+		struct pse51_mutexattr attr;
+	} shadow_mutex;
 };
 
 struct pse51_cond;
 
 union __xeno_cond {
-    pthread_cond_t native_cond;
-    struct __shadow_cond {
-	unsigned magic;
-	struct pse51_cond *cond;
-    } shadow_cond;
+	pthread_cond_t native_cond;
+	struct __shadow_cond {
+		unsigned magic;
+		struct pse51_cond *cond;
+	} shadow_cond;
 };
 
 struct pse51_interrupt;
@@ -194,6 +201,9 @@ struct pse51_interrupt;
 typedef struct pse51_interrupt *pthread_intr_t;
 
 #if defined(__KERNEL__) || defined(__XENO_SIM__)
+typedef struct pse51_mutexattr pthread_mutexattr_t;
+
+typedef struct pse51_condattr pthread_condattr_t;
 
 #ifdef __cplusplus
 extern "C" {
Index: ksrc/skins/posix/mutex.h
===================================================================
--- ksrc/skins/posix/mutex.h	(revision 3738)
+++ ksrc/skins/posix/mutex.h	(working copy)
@@ -21,6 +21,7 @@
 
 #include <posix/internal.h>
 #include <posix/thread.h>
+#include <posix/cb_lock.h>
 
 typedef struct pse51_mutex {
 	xnsynch_t synchbase;
@@ -29,47 +30,58 @@ typedef struct pse51_mutex {
 #define link2mutex(laddr)                                               \
 	((pse51_mutex_t *)(((char *)laddr) - offsetof(pse51_mutex_t, link)))
 
+	xnarch_atomic_intptr_t *owner;
 	pthread_mutexattr_t attr;
-	unsigned count;             /* lock count. */
-	unsigned condvars;          /* count of condition variables using this
-				       mutex. */
 	pse51_kqueues_t *owningq;
 } pse51_mutex_t;
 
+extern pthread_mutexattr_t pse51_default_mutex_attr;
+
 void pse51_mutexq_cleanup(pse51_kqueues_t *q);
 
 void pse51_mutex_pkg_init(void);
 
 void pse51_mutex_pkg_cleanup(void);
 
-/* Interruptible versions of pthread_mutex_*. Exposed for use by syscall.c. */
+/* Internal mutex functions, exposed for use by syscall.c. */
 int pse51_mutex_timedlock_break(struct __shadow_mutex *shadow,
 				int timed, xnticks_t to);
 
-/* must be called with nklock locked, interrupts off. */
-static inline int pse51_mutex_trylock_internal(xnthread_t *cur,
-					       struct __shadow_mutex *shadow,
-					       unsigned count)
+int pse51_mutex_check_init(struct __shadow_mutex *shadow,
+			   const pthread_mutexattr_t *attr);
+
+int pse51_mutex_init_internal(struct __shadow_mutex *shadow,
+			      pse51_mutex_t *mutex,
+			      xnarch_atomic_intptr_t *ownerp,
+			      const pthread_mutexattr_t *attr);
+
+void pse51_mutex_destroy_internal(pse51_mutex_t *mutex,
+				  pse51_kqueues_t *q);
+
+static inline xnthread_t *
+pse51_mutex_trylock_internal(xnthread_t *cur,
+			     struct __shadow_mutex *shadow, unsigned count)
 {
 	pse51_mutex_t *mutex = shadow->mutex;
+	xnthread_t *owner;
 
 	if (xnpod_unblockable_p())
-		return EPERM;
+		return ERR_PTR(-EPERM);
 
 	if (!pse51_obj_active(shadow, PSE51_MUTEX_MAGIC, struct __shadow_mutex))
-		return EINVAL;
+		return ERR_PTR(-EINVAL);
 
 #if XENO_DEBUG(POSIX)
 	if (mutex->owningq != pse51_kqueues(mutex->attr.pshared))
-		return EPERM;
+		return ERR_PTR(-EPERM);
 #endif /* XENO_DEBUG(POSIX) */
 
-	if (mutex->count)
-		return EBUSY;
+	owner = xnarch_atomic_intptr_cmpxchg(mutex->owner, NULL, cur);
+	if (unlikely(owner))
+		return owner;
 
-	xnsynch_set_owner(&mutex->synchbase, cur);
-	mutex->count = count;
-	return 0;
+	shadow->lockcnt = count;
+	return NULL;
 }
 
 /* must be called with nklock locked, interrupts off. */
@@ -81,31 +93,86 @@ static inline int pse51_mutex_timedlock_
 
 {
 	pse51_mutex_t *mutex;
+	xnthread_t *owner, *old;
+	spl_t s;
 	int err;
 
-	err = pse51_mutex_trylock_internal(cur, shadow, count);
-	if (err != EBUSY)
-		return err;
+  retry_lock:
+	owner = pse51_mutex_trylock_internal(cur, shadow, count);
+	if (likely(!owner) || IS_ERR(owner))
+		return PTR_ERR(owner);
 
 	mutex = shadow->mutex;
-	if (xnsynch_owner(&mutex->synchbase) == cur)
-		return EBUSY;
+	if (clear_claimed(owner) == cur)
+		return -EBUSY;
 
+	/* Set bit 0, so that mutex_unlock will know that the mutex is claimed.
+	   Hold the nklock, for mutual exclusion with slow mutex_unlock. */
+	xnlock_get_irqsave(&nklock, s);
+	while(!test_claimed(owner)) {
+		old = xnarch_atomic_intptr_cmpxchg(mutex->owner,
+						   owner, set_claimed(owner, 1));
+		if (likely(old == owner))
+			break;
+		if (old == NULL) {
+			/* Owner called fast mutex_unlock
+			   (on another cpu) */
+			xnlock_put_irqrestore(&nklock, s);
+			goto retry_lock;
+		}
+		owner = old;
+	}
+
+	xnsynch_set_owner(&mutex->synchbase, clear_claimed(owner));
 	if (timed)
 		xnsynch_sleep_on(&mutex->synchbase, abs_to, XN_REALTIME);
 	else
 		xnsynch_sleep_on(&mutex->synchbase, XN_INFINITE, XN_RELATIVE);
 
-	if (xnthread_test_info(cur, XNBREAK))
-		return EINTR;
-            
-	if (xnthread_test_info(cur, XNRMID))
-		return EINVAL;
-
-	if (xnthread_test_info(cur, XNTIMEO))
-		return ETIMEDOUT;
+	if (xnthread_test_info(cur, XNBREAK)) {
+		err = -EINTR;
+		goto error;
+	}
+	if (xnthread_test_info(cur, XNRMID)) {
+		err = -EINVAL;
+		goto error;
+	}
+	if (xnthread_test_info(cur, XNTIMEO)) {
+		err = -ETIMEDOUT;
+		goto error;
+	}
+
+	xnarch_atomic_intptr_set
+		(mutex->owner,
+		 set_claimed(cur, xnsynch_nsleepers(&mutex->synchbase)));
+	shadow->lockcnt = count;
+	xnlock_put_irqrestore(&nklock, s);
 
 	return 0;
+
+  error:
+	if (!xnsynch_nsleepers(&mutex->synchbase))
+		xnarch_atomic_intptr_set
+			(mutex->owner,
+			 clear_claimed(xnarch_atomic_intptr_get(mutex->owner)));
+	xnlock_put_irqrestore(&nklock, s);
+	return err;
+}
+
+static inline void pse51_mutex_unlock_internal(xnthread_t *cur,
+					       pse51_mutex_t *mutex)
+{
+	spl_t s;
+
+	if (likely(xnarch_atomic_intptr_cmpxchg(mutex->owner, cur, NULL) == cur))
+		return;
+
+	xnlock_get_irqsave(&nklock, s);
+	if (xnsynch_wakeup_one_sleeper(&mutex->synchbase))
+		xnpod_schedule();
+	else
+		xnarch_atomic_intptr_set(mutex->owner, NULL);
+	xnlock_put_irqrestore(&nklock, s);
 }
 
 #endif /* !_POSIX_MUTEX_H */
Index: ksrc/skins/posix/mutex.c
===================================================================
--- ksrc/skins/posix/mutex.c	(revision 3738)
+++ ksrc/skins/posix/mutex.c	(working copy)
@@ -47,23 +47,74 @@
  *
  *@{*/
 
+#include <nucleus/sys_ppd.h>
 #include <posix/mutex.h>
 
-static pthread_mutexattr_t default_attr;
+pthread_mutexattr_t pse51_default_mutex_attr;
 
-static void pse51_mutex_destroy_internal(pse51_mutex_t *mutex,
-					 pse51_kqueues_t *q)
+int pse51_mutex_check_init(struct __shadow_mutex *shadow,
+			   const pthread_mutexattr_t *attr)
 {
+	xnqueue_t *mutexq;
+
+	if (!attr)
+		attr = &pse51_default_mutex_attr;
+
+	mutexq = &pse51_kqueues(attr->pshared)->mutexq;
+
+	if (shadow->magic == PSE51_MUTEX_MAGIC) {
+		xnholder_t *holder;
+		for (holder = getheadq(mutexq); holder;
+		     holder = nextq(mutexq, holder))
+			if (holder == &shadow->mutex->link)
+				/* mutex is already in the queue. */
+				return -EBUSY;
+	}
+
+	return 0;
+}
+
+int pse51_mutex_init_internal(struct __shadow_mutex *shadow,
+			      pse51_mutex_t *mutex,
+			      xnarch_atomic_intptr_t *ownerp,
+			      const pthread_mutexattr_t *attr)
+{
+	xnflags_t synch_flags = XNSYNCH_PRIO | XNSYNCH_NOPIP;
+	struct xnsys_ppd *sys_ppd;
+	pse51_kqueues_t *kq;
 	spl_t s;
 
+	if (!attr)
+		attr = &pse51_default_mutex_attr;
+
+	if (attr->magic != PSE51_MUTEX_ATTR_MAGIC)
+		return -EINVAL;
+
+	kq = pse51_kqueues(attr->pshared);
+	sys_ppd = xnsys_ppd_get(attr->pshared);
+
+	shadow->magic = PSE51_MUTEX_MAGIC;
+	atomic_set(&shadow->lock, -1);
+	shadow->mutex = mutex;
+	shadow->attr = *attr;
+	shadow->lockcnt = 0;
+	shadow->owner_offset = xnheap_mapped_offset(&sys_ppd->sem_heap, ownerp);
+
+	if (attr->protocol == PTHREAD_PRIO_INHERIT)
+		synch_flags |= XNSYNCH_PIP;
+
+	xnsynch_init(&mutex->synchbase, synch_flags);
+	inith(&mutex->link);
+	mutex->attr = *attr;
+	mutex->owner = ownerp;
+	mutex->owningq = kq;
+	xnarch_atomic_intptr_set(ownerp, NULL);
+
 	xnlock_get_irqsave(&nklock, s);
-	removeq(&q->mutexq, &mutex->link);
-	/* synchbase wait queue may not be empty only when this function is called
-	   from pse51_mutex_pkg_cleanup, hence the absence of xnpod_schedule(). */
-	xnsynch_destroy(&mutex->synchbase);
+	appendq(&kq->mutexq, &mutex->link);
 	xnlock_put_irqrestore(&nklock, s);
 
-	xnfree(mutex);
+	return 0;
 }
 
 /**
@@ -83,72 +134,84 @@ static void pse51_mutex_destroy_internal
  * - EBUSY, the mutex @a mx was already initialized;
  * - ENOMEM, insufficient memory exists in the system heap to initialize the
  *   mutex, increase CONFIG_XENO_OPT_SYS_HEAPSZ.
+ * - EAGAIN, insufficient memory exists in the semaphore heap to initialize the
+ *   mutex, increase CONFIG_XENO_OPT_GLOBAL_SEM_HEAPSZ for a process-shared
+ *   mutex, or CONFG_XENO_OPT_SEM_HEAPSZ for a process-private mutex.
  *
  * @see
  * <a href="http://www.opengroup.org/onlinepubs/000095399/functions/pthread_mutex_init.html">
  * Specification.</a>
  * 
  */
-int pthread_mutex_init(pthread_mutex_t * mx, const pthread_mutexattr_t * attr)
+int pthread_mutex_init(pthread_mutex_t *mx, const pthread_mutexattr_t *attr)
 {
 	struct __shadow_mutex *shadow =
 	    &((union __xeno_mutex *)mx)->shadow_mutex;
-	xnflags_t synch_flags = XNSYNCH_PRIO | XNSYNCH_NOPIP;
+	DECLARE_CB_LOCK_FLAGS(s);
 	pse51_mutex_t *mutex;
-	xnqueue_t *mutexq;
-	spl_t s;
+	xnarch_atomic_intptr_t *ownerp;
 	int err;
 
 	if (!attr)
-		attr = &default_attr;
+		attr = &pse51_default_mutex_attr;
 
+	if (unlikely(cb_try_read_lock(&shadow->lock, s)))
+		goto checked;
+
+	err = pse51_mutex_check_init(shadow, attr);
+#ifndef XNARCH_HAVE_US_ATOMIC_CMPXCHG
+	cb_read_unlock(&shadow->lock, s);
+	if (err)
+		return -err;
+#else /* XNARCH_HAVE_US_ATOMIC_CMPXCHG */
+	if (err) {
+		cb_read_unlock(&shadow->lock, s);
+		return -err;
+	}
+#endif /* XNARCH_HAVE_US_ATOMIC_CMPXCHG */
+
+  checked:
 	mutex = (pse51_mutex_t *) xnmalloc(sizeof(*mutex));
 	if (!mutex)
 		return ENOMEM;
 
-	xnlock_get_irqsave(&nklock, s);
-
-	if (attr->magic != PSE51_MUTEX_ATTR_MAGIC) {
-		err = EINVAL;
-		goto error;
+	ownerp = (xnarch_atomic_intptr_t *)
+		xnheap_alloc(&xnsys_ppd_get(attr->pshared)->sem_heap,
+			     sizeof(xnarch_atomic_intptr_t));
+	if (!ownerp) {
+		xnfree(mutex);
+		return EAGAIN;
 	}
 
-	mutexq = &pse51_kqueues(attr->pshared)->mutexq;
-
-	if (shadow->magic == PSE51_MUTEX_MAGIC) {
-		xnholder_t *holder;
-		for (holder = getheadq(mutexq); holder;
-		     holder = nextq(mutexq, holder))
-			if (holder == &shadow->mutex->link) {
-				/* mutex is already in the queue. */
-				err = EBUSY;
-				goto error;
-			}
+	cb_force_write_lock(&shadow->lock, s);
+	err = pse51_mutex_init_internal(shadow, mutex, ownerp, attr);
+	cb_write_unlock(&shadow->lock, s);
+
+	if (err) {
+		xnfree(mutex);
+		xnheap_free(&xnsys_ppd_get(attr->pshared)->sem_heap, ownerp);
 	}
+	return -err;
+}
 
-	shadow->magic = PSE51_MUTEX_MAGIC;
-	shadow->mutex = mutex;
-
-	if (attr->protocol == PTHREAD_PRIO_INHERIT)
-		synch_flags |= XNSYNCH_PIP;
-
-	xnsynch_init(&mutex->synchbase, synch_flags);
-	inith(&mutex->link);
-	mutex->attr = *attr;
-	mutex->count = 0;
-	mutex->condvars = 0;
-	mutex->owningq = pse51_kqueues(attr->pshared);
-
-	appendq(mutexq, &mutex->link);
+void pse51_mutex_destroy_internal(pse51_mutex_t *mutex,
+				  pse51_kqueues_t *q)
+{
+	spl_t s;
 
+	xnlock_get_irqsave(&nklock, s);
+	removeq(&q->mutexq, &mutex->link);
+	/* synchbase wait queue may not be empty only when this function is called
+	   from pse51_mutex_pkg_cleanup, hence the absence of xnpod_schedule(). */
+	xnsynch_destroy(&mutex->synchbase);
 	xnlock_put_irqrestore(&nklock, s);
 
-	return 0;
-
-  error:
-	xnlock_put_irqrestore(&nklock, s);
+	if (mutex->attr.pshared)
+		xnheap_free(&xnsys_ppd_get(1)->sem_heap, mutex->owner);
+	/* We do not free the owner if the mutex is not pshared, because when
+	   this function is called from pse51_mutexq_cleanup, the sem_heap has
+	   been destroyed, and we have no way to find it back. */
 	xnfree(mutex);
-	return err;
 }
 
 /**
@@ -176,30 +239,34 @@ int pthread_mutex_destroy(pthread_mutex_
 {
 	struct __shadow_mutex *shadow =
 	    &((union __xeno_mutex *)mx)->shadow_mutex;
+	DECLARE_CB_LOCK_FLAGS(s);
 	pse51_mutex_t *mutex;
-	spl_t s;
 
-	xnlock_get_irqsave(&nklock, s);
+	if (unlikely(cb_try_write_lock(&shadow->lock, s)))
+		return EBUSY;
 
 	if (!pse51_obj_active(shadow, PSE51_MUTEX_MAGIC, struct __shadow_mutex)) {
-		xnlock_put_irqrestore(&nklock, s);
+		cb_write_unlock(&shadow->lock, s);
 		return EINVAL;
 	}
 
 	mutex = shadow->mutex;
 	if (pse51_kqueues(mutex->attr.pshared) != mutex->owningq) {
-		xnlock_put_irqrestore(&nklock, s);
+		cb_write_unlock(&shadow->lock, s);
 		return EPERM;
 	}
 
-	if (mutex->count || mutex->condvars) {
-		xnlock_put_irqrestore(&nklock, s);
+	if (xnarch_atomic_intptr_get(mutex->owner)) {
+		cb_write_unlock(&shadow->lock, s);
 		return EBUSY;
 	}
 
 	pse51_mark_deleted(shadow);
-	xnlock_put_irqrestore(&nklock, s);
+	cb_write_unlock(&shadow->lock, s);
 
+	if (!mutex->attr.pshared)
+		xnheap_free(&xnsys_ppd_get(mutex->attr.pshared)->sem_heap,
+			    mutex->owner);
 	pse51_mutex_destroy_internal(mutex, pse51_kqueues(mutex->attr.pshared));
 	
 	return 0;
@@ -210,20 +277,19 @@ int pse51_mutex_timedlock_break(struct _
 {
 	xnthread_t *cur = xnpod_current_thread();
 	pse51_mutex_t *mutex;
-	int err;
 	spl_t s;
-
-	xnlock_get_irqsave(&nklock, s);
+	int err;
 
 	err = pse51_mutex_timedlock_internal(cur, shadow, 1, timed, abs_to);
-	if (err != EBUSY)
+	if (err != -EBUSY)
 		goto unlock_and_return;
 
 	mutex = shadow->mutex;
 
-	switch (mutex->attr.type) {
+	switch(mutex->attr.type) {
 	case PTHREAD_MUTEX_NORMAL:
 		/* Attempting to relock a normal mutex, deadlock. */
+		xnlock_get_irqsave(&nklock, s);
 		for (;;) {
 			if (timed)
 				xnsynch_sleep_on(&mutex->synchbase,
@@ -233,41 +299,41 @@ int pse51_mutex_timedlock_break(struct _
 						 XN_INFINITE, XN_RELATIVE);
 
 			if (xnthread_test_info(cur, XNBREAK)) {
-				err = EINTR;
+				err = -EINTR;
 				break;
 			}
 
 			if (xnthread_test_info(cur, XNTIMEO)) {
-				err = ETIMEDOUT;
+				err = -ETIMEDOUT;
 				break;
 			}
 
 			if (xnthread_test_info(cur, XNRMID)) {
-				err = EINVAL;
+				err = -EINVAL;
 				break;
 			}
 		}
+		xnlock_put_irqrestore(&nklock, s);
 
 		break;
 
 	case PTHREAD_MUTEX_ERRORCHECK:
-		err = EDEADLK;
+		err = -EDEADLK;
 		break;
 
 	case PTHREAD_MUTEX_RECURSIVE:
-		if (mutex->count == UINT_MAX) {
-			err = EAGAIN;
+		if (shadow->lockcnt == UINT_MAX) {
+			err = -EAGAIN;
 			break;
 		}
 
-		++mutex->count;
+		++shadow->lockcnt;
 		err = 0;
 	}
 
   unlock_and_return:
-	xnlock_put_irqrestore(&nklock, s);
-
 	return err;
+		
 }
 
 /**
@@ -298,33 +364,36 @@ int pse51_mutex_timedlock_break(struct _
  * Specification.</a>
  * 
  */
-int pthread_mutex_trylock(pthread_mutex_t * mx)
+int pthread_mutex_trylock(pthread_mutex_t *mx)
 {
 	struct __shadow_mutex *shadow =
 	    &((union __xeno_mutex *)mx)->shadow_mutex;
-	xnthread_t *cur = xnpod_current_thread();
+	xnthread_t *owner, *cur = xnpod_current_thread();
+	DECLARE_CB_LOCK_FLAGS(s);
 	int err;
-	spl_t s;
 
-	xnlock_get_irqsave(&nklock, s);
+	if (unlikely(cb_try_read_lock(&shadow->lock, s)))
+		return EINVAL;
 
-	err = pse51_mutex_trylock_internal(cur, shadow, 1);
+	owner = pse51_mutex_trylock_internal(cur, shadow, 1);
+	if (likely(!owner) || IS_ERR(owner))
+		return -PTR_ERR(owner);
 
-	if (err == EBUSY) {
+	err = EBUSY;
+	if (clear_claimed(owner) == cur) {
 		pse51_mutex_t *mutex = shadow->mutex;
 
-		if (mutex->attr.type == PTHREAD_MUTEX_RECURSIVE
-		    && xnsynch_owner(&mutex->synchbase) == cur) {
-			if (mutex->count == UINT_MAX)
+		if (mutex->attr.type == PTHREAD_MUTEX_RECURSIVE) {
+			if (shadow->lockcnt == UINT_MAX)
 				err = EAGAIN;
 			else {
-				++mutex->count;
+				++shadow->lockcnt;
 				err = 0;
 			}
 		}
 	}
 
-	xnlock_put_irqrestore(&nklock, s);
+	cb_read_unlock(&shadow->lock, s);
 
 	return err;
 }
@@ -369,12 +438,18 @@ int pthread_mutex_lock(pthread_mutex_t *
 {
 	struct __shadow_mutex *shadow =
 	    &((union __xeno_mutex *)mx)->shadow_mutex;
+	DECLARE_CB_LOCK_FLAGS(s);
 	int err;
 
+	if (unlikely(cb_try_read_lock(&shadow->lock, s)))
+		return EINVAL;
+
 	do {
 		err = pse51_mutex_timedlock_break(shadow, 0, XN_INFINITE);
 	} while (err == EINTR);
 
+	cb_read_unlock(&shadow->lock, s);
+
 	return err;
 }
 
@@ -416,40 +491,20 @@ int pthread_mutex_timedlock(pthread_mute
 {
 	struct __shadow_mutex *shadow =
 	    &((union __xeno_mutex *)mx)->shadow_mutex;
+	DECLARE_CB_LOCK_FLAGS(s);
 	int err;
 
+	if (unlikely(cb_try_read_lock(&shadow->lock, s)))
+		return EINVAL;
+
 	do {
 		err = pse51_mutex_timedlock_break(shadow, 1,
 						  ts2ticks_ceil(to) + 1);
 	} while (err == EINTR);
 
-	return err;
-}
-
-/* must be called with nklock locked, interrupts off.
-
-   Note: the function mutex_save_count() in cond.c is very similar to this
-   function.
-*/
-static inline int mutex_unlock_internal(xnthread_t *cur,
-					struct __shadow_mutex *shadow)
-{
-	pse51_mutex_t *mutex;
-
-	if (!pse51_obj_active(shadow, PSE51_MUTEX_MAGIC, struct __shadow_mutex))
-		 return EINVAL;
-
-	mutex = shadow->mutex;
-
-	if (xnsynch_owner(&mutex->synchbase) != cur || mutex->count != 1)
-		return EPERM;
-
-	if (xnsynch_wakeup_one_sleeper(&mutex->synchbase))
-		xnpod_schedule();
-	else
-		mutex->count = 0;
+	cb_read_unlock(&shadow->lock, s);
 
-	return 0;
+	return err;
 }
 
 /**
@@ -488,28 +543,41 @@ int pthread_mutex_unlock(pthread_mutex_t
 	struct __shadow_mutex *shadow =
 	    &((union __xeno_mutex *)mx)->shadow_mutex;
 	xnthread_t *cur = xnpod_current_thread();
+	DECLARE_CB_LOCK_FLAGS(s);
+	pse51_mutex_t *mutex;
 	int err;
-	spl_t s;
 
 	if (xnpod_root_p() || xnpod_interrupt_p())
-		return EPERM;
+		return -EPERM;
 
-	xnlock_get_irqsave(&nklock, s);
+	if (unlikely(cb_try_read_lock(&shadow->lock, s)))
+		return EINVAL;
 
-	err = mutex_unlock_internal(cur, shadow);
+	if (!pse51_obj_active(shadow,
+			      PSE51_MUTEX_MAGIC, struct __shadow_mutex)) {
+		err = EINVAL;
+		goto out;
+	}
 
-	if (err == EPERM) {
-		pse51_mutex_t *mutex = shadow->mutex;
+	mutex = shadow->mutex;
+	
+	if (clear_claimed(xnarch_atomic_intptr_get(mutex->owner)) != cur) {
+		err = EPERM;
+		goto out;
+	}
 
-		if (mutex->attr.type == PTHREAD_MUTEX_RECURSIVE
-		    && xnsynch_owner(&mutex->synchbase) == cur
-		    && mutex->count) {
-			--mutex->count;
-			err = 0;
-		}
+	err = 0;
+	if (shadow->lockcnt > 1) {
+		/* Mutex is recursive */
+		--shadow->lockcnt;
+		cb_read_unlock(&shadow->lock, s);
+		return 0;
 	}
 
-	xnlock_put_irqrestore(&nklock, s);
+	pse51_mutex_unlock_internal(cur, mutex);
+
+  out:
+	cb_read_unlock(&shadow->lock, s);
 
 	return err;
 }
@@ -536,7 +604,7 @@ void pse51_mutexq_cleanup(pse51_kqueues_
 void pse51_mutex_pkg_init(void)
 {
 	initq(&pse51_global_kqueues.mutexq);
-	pthread_mutexattr_init(&default_attr);
+	pthread_mutexattr_init(&pse51_default_mutex_attr);
 }
 
 void pse51_mutex_pkg_cleanup(void)
Index: ksrc/skins/posix/cond.c
===================================================================
--- ksrc/skins/posix/cond.c	(revision 3738)
+++ ksrc/skins/posix/cond.c	(working copy)
@@ -229,15 +229,16 @@ static inline int mutex_save_count(xnthr
 
 	mutex = shadow->mutex;
 
-	if (xnsynch_owner(&mutex->synchbase) != cur || mutex->count == 0)
+	if (clear_claimed(xnarch_atomic_intptr_get(mutex->owner)) != cur)
 		return EPERM;
 
-	*count_ptr = mutex->count;
+	*count_ptr = shadow->lockcnt;
 
-	if (xnsynch_wakeup_one_sleeper(&mutex->synchbase))
-		mutex->count = 1;
-	else
-		mutex->count = 0;
+	if (likely(xnarch_atomic_intptr_cmpxchg(mutex->owner, cur, NULL) == cur))
+		return 0;
+
+	if (!xnsynch_wakeup_one_sleeper(&mutex->synchbase))
+		xnarch_atomic_intptr_set(mutex->owner, NULL);
 	/* Do not reschedule here, releasing the mutex and suspension must be
 	   done atomically in pthread_cond_*wait. */
 
@@ -287,10 +288,8 @@ int pse51_cond_timedwait_prologue(xnthre
 		goto unlock_and_return;
 
 	/* Bind mutex to cond. */
-	if (cond->mutex == NULL) {
+	if (cond->mutex == NULL)
 		cond->mutex = mutex->mutex;
-		++mutex->mutex->condvars;
-	}
 
 	/* Wait for another thread to signal the condition. */
 	if (timed)
@@ -349,11 +348,8 @@ int pse51_cond_timedwait_epilogue(xnthre
 	/* Unbind mutex and cond, if no other thread is waiting, if the job was
 	   not already done. */
 	if (!xnsynch_nsleepers(&cond->synchbase)
-	    && cond->mutex == mutex->mutex) {
-	
-		--mutex->mutex->condvars;
+	    && cond->mutex == mutex->mutex)
 		cond->mutex = NULL;
-	}
 
 	thread_cancellation_point(cur);
 
@@ -419,9 +415,15 @@ int pthread_cond_wait(pthread_cond_t * c
 	struct __shadow_mutex *mutex =
 	    &((union __xeno_mutex *)mx)->shadow_mutex;
 	xnthread_t *cur = xnpod_current_thread();
+	DECLARE_CB_LOCK_FLAGS(s);
 	unsigned count;
 	int err;
 
+#ifdef XNARCH_HAVE_US_ATOMIC_CMPXCHG
+	if (unlikely(cb_try_read_lock(&mutex->lock, s)))
+		return EINVAL;
+#endif /* XNARCH_HAVE_US_ATOMIC_CMPXCHG */
+
 	err = pse51_cond_timedwait_prologue(cur, cond, mutex,
 					    &count, 0, XN_INFINITE);
 
@@ -430,6 +432,10 @@ int pthread_cond_wait(pthread_cond_t * c
 							      mutex, count))
 			;
 
+#ifdef XNARCH_HAVE_US_ATOMIC_CMPXCHG
+	cb_read_unlock(&mutex->lock, s);
+#endif /* XNARCH_HAVE_US_ATOMIC_CMPXCHG */
+
 	return err != EINTR ? err : 0;
 }
 
@@ -481,6 +487,11 @@ int pthread_cond_timedwait(pthread_cond_
 	unsigned count;
 	int err;
 
+#ifdef XNARCH_HAVE_US_ATOMIC_CMPXCHG
+	if (unlikely(cb_try_read_lock(&mutex->lock, s)))
+		return EINVAL;
+#endif /* XNARCH_HAVE_US_ATOMIC_CMPXCHG */
+
 	err = pse51_cond_timedwait_prologue(cur, cond, mutex, &count, 1,
 					    ts2ticks_ceil(abstime) + 1);
 
@@ -489,6 +500,10 @@ int pthread_cond_timedwait(pthread_cond_
 							      mutex, count))
 			;
 
+#ifdef XNARCH_HAVE_US_ATOMIC_CMPXCHG
+	cb_read_unlock(&mutex->lock, s);
+#endif /* XNARCH_HAVE_US_ATOMIC_CMPXCHG */
+
 	return err != EINTR ? err : 0;
 }
 
Index: ksrc/skins/posix/syscall.c
===================================================================
--- ksrc/skins/posix/syscall.c	(revision 3738)
+++ ksrc/skins/posix/syscall.c	(working copy)
@@ -23,6 +23,7 @@
 #include <asm/xenomai/wrappers.h>
 #include <nucleus/jhash.h>
 #include <nucleus/ppd.h>
+#include <nucleus/sys_ppd.h>
 #include <posix/syscall.h>
 #include <posix/posix.h>
 #include <posix/thread.h>
@@ -884,6 +885,7 @@ static int __pthread_mutexattr_setpshare
 	return __xn_safe_copy_to_user((void __user *)uattrp, &attr, sizeof(*uattrp));
 }
 
+#ifndef XNARCH_HAVE_US_ATOMIC_CMPXCHG
 static int __pthread_mutex_init(struct pt_regs *regs)
 {
 	pthread_mutexattr_t locattr, *attr, *uattrp;
@@ -941,54 +943,293 @@ static int __pthread_mutex_destroy(struc
 static int __pthread_mutex_lock(struct pt_regs *regs)
 {
 	union __xeno_mutex mx, *umx;
+	DECLARE_CB_LOCK_FLAGS(s);
+	int err;
 
 	umx = (union __xeno_mutex *)__xn_reg_arg1(regs);
 
 	if (__xn_safe_copy_from_user(&mx.shadow_mutex,
 				     (void __user *)&umx->shadow_mutex,
-				     sizeof(mx.shadow_mutex)))
+				     offsetof(struct __shadow_mutex, lock)))
+		return -EFAULT;
+
+	if (unlikely(cb_try_read_lock(&mx.shadow_mutex.lock, s)))
+		return -EINVAL;
+
+	err = pse51_mutex_timedlock_break(&mx.shadow_mutex, 0, XN_INFINITE);
+
+	cb_read_unlock(&mx.shadow_mutex.lock);
+
+	if (!err &&
+	    __xn_safe_copy_to_user((void __user *)&umx->shadow_mutex.lockcnt,
+				   &mx.shadow_mutex.lockcnt,
+				   sizeof(umx->shadow_mutex.lockcnt)))
 		return -EFAULT;
 
-	return -pse51_mutex_timedlock_break(&mx.shadow_mutex, 0, XN_INFINITE);
+	return -err
 }
 
 static int __pthread_mutex_timedlock(struct pt_regs *regs)
 {
 	union __xeno_mutex mx, *umx;
+	DECLARE_CB_LOCK_FLAGS(s);
 	struct timespec ts;
+	int err;
 
 	umx = (union __xeno_mutex *)__xn_reg_arg1(regs);
 
 	if (__xn_safe_copy_from_user(&mx.shadow_mutex,
 				     (void __user *)&umx->shadow_mutex,
-				     sizeof(mx.shadow_mutex)))
+				     offsetof(struct __shadow_mutex, lock)))
 		return -EFAULT;
 
 	if (__xn_safe_copy_from_user(&ts,
 				     (void __user *)__xn_reg_arg2(regs), sizeof(ts)))
 		return -EFAULT;
 
-	return -pse51_mutex_timedlock_break(&mx.shadow_mutex,
-					    1, ts2ticks_ceil(&ts) + 1);
+	if (unlikely(cb_try_read_lock(&mx.shadow_mutex.lock, s)))
+		return -EINVAL;
+
+	err = pse51_mutex_timedlock_break(&mx.shadow_mutex,
+					  1, ts2ticks_ceil(&ts) + 1);
+
+	cb_read_unlock(&mx.shadow_mutex.lock);
+
+	if (!err &&
+	    __xn_safe_copy_to_user((void __user *)&umx->shadow_mutex.lockcnt,
+				   &mx.shadow_mutex.lockcnt,
+				   sizeof(umx->shadow_mutex.lockcnt)))
+		return -EFAULT;
+
+	return -err
 }
 
 static int __pthread_mutex_trylock(struct pt_regs *regs)
 {
 	union __xeno_mutex mx, *umx;
+	int err;
 
 	umx = (union __xeno_mutex *)__xn_reg_arg1(regs);
 
 	if (__xn_safe_copy_from_user(&mx.shadow_mutex,
 				     (void __user *)&umx->shadow_mutex,
-				     sizeof(mx.shadow_mutex)))
+				     offsetof(struct __shadow_mutex, lock)))
 		return -EFAULT;
 
-	return -pthread_mutex_trylock(&mx.native_mutex);
+	err = pthread_mutex_trylock(&mx.native_mutex);
+
+	if (!err &&
+	    __xn_safe_copy_to_user((void __user *)&umx->shadow_mutex.lockcnt,
+				   &mx.shadow_mutex.lockcnt,
+				   sizeof(umx->shadow_mutex.lockcnt)))
+		return -EFAULT;
+
+	return -err;
 }
 
 static int __pthread_mutex_unlock(struct pt_regs *regs)
 {
+	xnthread_t *cur = xnpod_current_thread();
+	struct __shadow_mutex *shadow;
+	union __xeno_mutex mx, *umx;
+	DECLARE_CB_LOCK_FLAGS(s);
+	pse51_mutex_t *mutex;
+	int err;
+
+	if (xnpod_root_p())
+		return -EPERM;
+
+	umx = (union __xeno_mutex *)__xn_reg_arg1(regs);
+
+	if (__xn_safe_copy_from_user(&mx.shadow_mutex,
+				     (void __user *)&umx->shadow_mutex,
+				     offsetof(struct __shadow_mutex, lock)))
+		return -EFAULT;
+
+	shadow = &mx.shadow_mutex;
+
+	if (unlikely(cb_try_read_lock(&shadow->lock, s)))
+		return -EINVAL;
+
+	if (!pse51_obj_active(shadow,
+			      PSE51_MUTEX_MAGIC, struct __shadow_mutex)) {
+		err = -EINVAL;
+		goto out;
+	}
+
+	mutex = shadow->mutex;
+
+	if (clear_claimed(xnarch_atomic_intptr_get(mutex->owner)) != cur) {
+		err = -EPERM;
+		goto out;
+	}
+
+	err = 0;
+	if (shadow->lockcnt > 1) {
+		/* Mutex is recursive */
+		--shadow->lockcnt;
+		cb_read_unlock(&shadow->lock, s);
+
+		if (__xn_safe_copy_to_user((void __user *)
+					   &umx->shadow_mutex.lockcnt,
+					   &shadow->lockcnt,
+					   sizeof(umx->shadow_mutex.lockcnt)))
+			return -EFAULT;
+
+		return 0;
+	}
+	
+	pse51_mutex_unlock_internal(cur, mutex);
+
+  out:
+	cb_read_unlock(&shadow->lock, s);
+
+	return err;
+}
+#else /* !XNARCH_HAVE_US_ATOMIC_CMPXCHG */
+static int __pthread_mutex_check_init(struct pt_regs *regs)
+{
+	pthread_mutexattr_t locattr, *attr, *uattrp;
+	union __xeno_mutex mx, *umx;
+
+	umx = (union __xeno_mutex *)__xn_reg_arg1(regs);
+
+	uattrp = (pthread_mutexattr_t *) __xn_reg_arg2(regs);
+
+	if (__xn_safe_copy_from_user(&mx.shadow_mutex,
+				     (void __user *)&umx->shadow_mutex,
+				     sizeof(mx.shadow_mutex)))
+		return -EFAULT;
+
+	if (uattrp) {
+		if (__xn_safe_copy_from_user(&locattr, (void __user *)
+					     uattrp, sizeof(locattr)))
+			return -EFAULT;
+
+		attr = &locattr;
+	} else
+		attr = NULL;
+
+	return pse51_mutex_check_init(&umx->shadow_mutex, attr);
+}
+
+static int __pthread_mutex_init(struct pt_regs *regs)
+{
+	pthread_mutexattr_t locattr, *attr, *uattrp;
+	union __xeno_mutex mx, *umx;
+	pse51_mutex_t *mutex;
+	xnarch_atomic_intptr_t *ownerp;
+	int err;
+
+	umx = (union __xeno_mutex *)__xn_reg_arg1(regs);
+
+	uattrp = (pthread_mutexattr_t *) __xn_reg_arg2(regs);
+
+	if (__xn_safe_copy_from_user(&mx.shadow_mutex,
+				     (void __user *)&umx->shadow_mutex,
+				     sizeof(mx.shadow_mutex)))
+		return -EFAULT;
+
+	if (uattrp) {
+		if (__xn_safe_copy_from_user(&locattr, (void __user *)
+					     uattrp, sizeof(locattr)))
+			return -EFAULT;
+
+		attr = &locattr;
+	} else
+		attr = &pse51_default_mutex_attr;
+
+	mutex = (pse51_mutex_t *) xnmalloc(sizeof(*mutex));
+	if (!mutex)
+		return -ENOMEM;
+
+	ownerp = (xnarch_atomic_intptr_t *)
+		xnheap_alloc(&xnsys_ppd_get(attr->pshared)->sem_heap,
+			     sizeof(xnarch_atomic_intptr_t));
+	if (!ownerp) {
+		xnfree(mutex);
+		return -EAGAIN;
+	}
+
+	err = pse51_mutex_init_internal(&mx.shadow_mutex, mutex, ownerp, attr);
+	if (err) {
+		xnfree(mutex);
+		xnheap_free(&xnsys_ppd_get(attr->pshared)->sem_heap, ownerp);
+		return err;
+	}
+
+	return __xn_safe_copy_to_user((void __user *)&umx->shadow_mutex,
+				      &mx.shadow_mutex, sizeof(umx->shadow_mutex));
+}
+
+static int __pthread_mutex_destroy(struct pt_regs *regs)
+{
+	struct __shadow_mutex *shadow;
 	union __xeno_mutex mx, *umx;
+	pse51_mutex_t *mutex;
+
+	umx = (union __xeno_mutex *)__xn_reg_arg1(regs);
+
+	shadow = &mx.shadow_mutex;
+
+	if (__xn_safe_copy_from_user(shadow,
+				     (void __user *)&umx->shadow_mutex,
+				     sizeof(*shadow)))
+		return -EFAULT;
+
+	if (!pse51_obj_active(shadow, PSE51_MUTEX_MAGIC, struct __shadow_mutex))
+		return -EINVAL;
+
+	mutex = shadow->mutex;
+	if (pse51_kqueues(mutex->attr.pshared) != mutex->owningq)
+		return -EPERM;
+
+	if (xnarch_atomic_intptr_get(mutex->owner))
+		return -EBUSY;
+
+	pse51_mark_deleted(shadow);
+	if (!mutex->attr.pshared)
+		xnheap_free(&xnsys_ppd_get(mutex->attr.pshared)->sem_heap,
+			    mutex->owner);
+	pse51_mutex_destroy_internal(mutex, mutex->owningq);
+
+	return __xn_safe_copy_to_user((void __user *)&umx->shadow_mutex,
+				      shadow, sizeof(umx->shadow_mutex));
+}
+
+static int __pthread_mutex_lock(struct pt_regs *regs)
+{
+	struct __shadow_mutex *shadow;
+	union __xeno_mutex mx, *umx;
+	int err;
+
+	umx = (union __xeno_mutex *)__xn_reg_arg1(regs);
+
+	if (__xn_safe_copy_from_user(&mx.shadow_mutex,
+				     (void __user *)&umx->shadow_mutex,
+				     offsetof(struct __shadow_mutex, lock)))
+		return -EFAULT;
+
+	shadow = &mx.shadow_mutex;
+
+	err = pse51_mutex_timedlock_break(&mx.shadow_mutex, 0, XN_INFINITE);
+
+	if (!err &&
+	    __xn_safe_copy_to_user((void __user *)
+				   &umx->shadow_mutex.lockcnt,
+				   &shadow->lockcnt,
+				   sizeof(umx->shadow_mutex.lockcnt)))
+		return -EFAULT;
+
+	return -err;
+}
+
+static int __pthread_mutex_timedlock(struct pt_regs *regs)
+{
+	struct __shadow_mutex *shadow;
+	union __xeno_mutex mx, *umx;
+	struct timespec ts;
+	int err;
 
 	umx = (union __xeno_mutex *)__xn_reg_arg1(regs);
 
@@ -997,8 +1238,46 @@ static int __pthread_mutex_unlock(struct
 				     sizeof(mx.shadow_mutex)))
 		return -EFAULT;
 
-	return -pthread_mutex_unlock(&mx.native_mutex);
+	if (__xn_safe_copy_from_user(&ts,
+				     (void __user *)__xn_reg_arg2(regs),
+				     sizeof(ts)))
+		return -EFAULT;
+
+	shadow = &mx.shadow_mutex;
+
+	err = pse51_mutex_timedlock_break(&mx.shadow_mutex,
+					    1, ts2ticks_ceil(&ts) + 1);
+
+	if (!err &&
+	    __xn_safe_copy_to_user((void __user *)
+				   &umx->shadow_mutex.lockcnt,
+				   &shadow->lockcnt,
+				   sizeof(umx->shadow_mutex.lockcnt)))
+		return -EFAULT;
+
+	return -err;
+}
+
+static int __pthread_mutex_unlock(struct pt_regs *regs)
+{
+	xnthread_t *cur = xnpod_current_thread();
+	union __xeno_mutex mx, *umx;
+
+	if (xnpod_root_p())
+		return -EPERM;
+
+	umx = (union __xeno_mutex *)__xn_reg_arg1(regs);
+
+	if (__xn_safe_copy_from_user(&mx.shadow_mutex,
+				     (void __user *)&umx->shadow_mutex,
+				     offsetof(struct __shadow_mutex, lock)))
+		return -EFAULT;
+
+	pse51_mutex_unlock_internal(cur, mx.shadow_mutex.mutex);
+
+	return 0;
 }
+#endif /* !XNARCH_HAVE_US_ATOMIC_CMPXCHG */
 
 static int __pthread_condattr_init(struct pt_regs *regs)
 {
@@ -2394,7 +2673,11 @@ static xnsysent_t __systab[] = {
 	[__pse51_mutex_lock] = {&__pthread_mutex_lock, __xn_exec_primary},
 	[__pse51_mutex_timedlock] =
 	    {&__pthread_mutex_timedlock, __xn_exec_primary},
+#ifndef XNARCH_HAVE_US_ATOMIC_CMPXCHG
 	[__pse51_mutex_trylock] = {&__pthread_mutex_trylock, __xn_exec_primary},
+#else
+        [__pse51_check_init] = {&__pthread_mutex_check_init, __xn_exec_any},
+#endif
 	[__pse51_mutex_unlock] = {&__pthread_mutex_unlock, __xn_exec_primary},
 	[__pse51_cond_init] = {&__pthread_cond_init, __xn_exec_any},
 	[__pse51_cond_destroy] = {&__pthread_cond_destroy, __xn_exec_any},
@@ -2477,7 +2760,6 @@ static void *pse51_eventcb(int event, vo
 
 	switch (event) {
 	case XNSHADOW_CLIENT_ATTACH:
-
 		q = (pse51_queues_t *) xnarch_alloc_host_mem(sizeof(*q));
 		if (!q)
 			return ERR_PTR(-ENOSPC);


-- 


					    Gilles.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [Xenomai-core] [Patch 7/7] Re-implementation of mutexes, user-space support.
  2008-05-02 22:36           ` [Xenomai-core] [Patch 6/7] Re-implementation of mutexes, kernel-space support Gilles Chanteperdrix
@ 2008-05-02 22:38             ` Gilles Chanteperdrix
  2008-05-18 16:42               ` Philippe Gerum
  0 siblings, 1 reply; 34+ messages in thread
From: Gilles Chanteperdrix @ 2008-05-02 22:38 UTC (permalink / raw)
  To: xenomai


Since binding of the semaphore heaps is now made by xeno_skin_bind, there is
much less modifications in src/skins/posix/init.c. However, I had to do
something really ugly: since binding the semaphore heaps by xeno_skin_bind
requires calls to open, ioctl, mmap, close and munmap, I redefined these symbols
to be the __real_ variants before including bind.h.
Another solution would have been to use __open (which would not have been
wrapped) instead of open in bind.h and rely on the fact that the posix skin
wrapped syscalls fall back to the __real variants when called for non real-time
file descriptors. I do not know however, if this solution would have been
portable. In particular, does uclibc also defines __open ?

---
 Makefile.am |    2
 cond.c      |   21 ++++
 init.c      |   26 +++++
 mutex.c     |  265 ++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 4 files changed, 287 insertions(+), 27 deletions(-)

Index: src/skins/posix/init.c
===================================================================
--- src/skins/posix/init.c	(revision 3738)
+++ src/skins/posix/init.c	(working copy)
@@ -26,9 +26,23 @@
 #include <posix/posix.h>
 #include <posix/syscall.h>
 #include <rtdm/syscall.h>
-#include <asm-generic/bits/bind.h>
 #include <asm-generic/bits/mlock_alert.h>
 
+/* asm-generic/bits/bind.h uses the following functions, so we redefine them to
+   be the __real variants */
+#undef open
+#undef ioctl
+#undef mmap
+#undef close
+#undef munmap
+#define open __real_open
+#define ioctl __real_ioctl
+#define mmap __real_mmap
+#define close __real_close
+#define munmap __real_munmap
+
+#include <asm-generic/bits/bind.h>
+
 int __pse51_muxid = -1;
 int __rtdm_muxid = -1;
 int __rtdm_fd_start = INT_MAX;
@@ -91,5 +105,15 @@ void __init_posix_interface(void)
 			exit(EXIT_FAILURE);
 		}
 		fork_handler_registered = 1;
+
+		if (sizeof(struct __shadow_mutex) > sizeof(pthread_mutex_t)) {
+			fprintf(stderr, "sizeof(pthread_mutex_t): %d,"
+				" sizeof(shadow_mutex): %d\n",
+				sizeof(pthread_mutex_t),
+				sizeof(struct __shadow_mutex));
+			exit(EXIT_FAILURE);
+		}
+
 	}
 }
+
Index: src/skins/posix/mutex.c
===================================================================
--- src/skins/posix/mutex.c	(revision 3738)
+++ src/skins/posix/mutex.c	(working copy)
@@ -17,10 +17,15 @@
  */
 
 #include <errno.h>
-#include <posix/syscall.h>
 #include <pthread.h>
+#include <posix/syscall.h>
+#include <posix/cb_lock.h>
+#include <asm-generic/bits/current.h>
+
+#define PSE51_MUTEX_MAGIC (0x86860303)
 
 extern int __pse51_muxid;
+extern unsigned long xeno_sem_heap[2];
 
 int __wrap_pthread_mutexattr_init(pthread_mutexattr_t *attr)
 {
@@ -73,66 +78,280 @@ 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 xnarch_atomic_intptr_t *get_ownerp(struct __shadow_mutex *shadow)
+{
+	if (likely(!shadow->attr.pshared))
+		return shadow->owner;
+	
+	return (xnarch_atomic_intptr_t *) (xeno_sem_heap[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 = (xnarch_atomic_intptr_t *)
+			(xeno_sem_heap[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;
+
+	if (unlikely(cb_try_write_lock(&shadow->lock, s)))
+		return EINVAL;
 
-	return -XENOMAI_SKINCALL1(__pse51_muxid,
-				  __pse51_mutex_destroy, &_mutex->shadow_mutex);
+	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 = xeno_get_current();
+	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 = xnarch_atomic_intptr_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 = xeno_get_current();
+	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 = xnarch_atomic_intptr_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 = xeno_get_current();
+	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 = xnarch_atomic_intptr_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 -XENOMAI_SKINCALL1(__pse51_muxid,
-				  __pse51_mutex_trylock, &_mutex->shadow_mutex);
+	return -err;
+
+#else /* !XNARCH_HAVE_US_ATOMIC_CMPXCHG */
+
+	return -XENOMAI_SKINCALL1(__pse51_muxid, __pse51_mutex_trylock, shadow);
+
+#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
+	xnarch_atomic_intptr_t *ownerp;
+	xnthread_t *cur;
+	int err = 0;
+
+	cur = xeno_get_current();
+	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(xnarch_atomic_intptr_get(ownerp)) != cur)) {
+		err = -EPERM;
+		goto out_err;
+	}
+
+	err = 0;
+	if (shadow->lockcnt > 1) {
+		--shadow->lockcnt;
+		goto out;
+	}
+
+	if (likely(xnarch_atomic_intptr_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 3738)
+++ 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 3738)
+++ 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.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Xenomai-core] [Patch 0/7] Posix skin user-space mutexes, second take.
  2008-05-02 22:27 [Xenomai-core] [Patch 0/7] Posix skin user-space mutexes, second take Gilles Chanteperdrix
  2008-05-02 22:30 ` [Xenomai-core] [Patch 1/7] Support for non cached memory mappings Gilles Chanteperdrix
@ 2008-05-03 19:18 ` Gilles Chanteperdrix
  1 sibling, 0 replies; 34+ messages in thread
From: Gilles Chanteperdrix @ 2008-05-03 19:18 UTC (permalink / raw)
  To: xenomai

Gilles Chanteperdrix wrote:
 > 
 > Hi,
 > 
 > here comes a second attempt of implementing user-space mutexes for the posix
 > skin.
 > 
 > Only differences with the first implementation are explained in the following
 > mails.
 > 
 > Thanks in advance for your review.

After extensive testing, except for bugs in condition variables system
calls, the new implementation seems to work fine.

Of course, it is well known that the one who makes the test should not be
the same as the one who makes the code...

-- 


					    Gilles.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Xenomai-core] [Patch 4/7] Define ARM atomic operations in user-space
  2008-05-02 22:34       ` [Xenomai-core] [Patch 4/7] Define ARM " Gilles Chanteperdrix
  2008-05-02 22:35         ` [Xenomai-core] [Patch 5/7] Define new syscalls for the system skin Gilles Chanteperdrix
@ 2008-05-05 16:33         ` Gilles Chanteperdrix
  2008-05-05 16:39           ` Philippe Gerum
  1 sibling, 1 reply; 34+ messages in thread
From: Gilles Chanteperdrix @ 2008-05-05 16:33 UTC (permalink / raw)
  To: xenomai

On Sat, May 3, 2008 at 12:34 AM, Gilles Chanteperdrix
<gilles.chanteperdrix@xenomai.org> wrote:
>
>  The include/asm-arm/atomic.h header now defines the xnarch_memory_barrier in
>  addition to user-space atomic operations. The pxa3xx deserves a special
>  treatment since it uses the ARMv6 memory barrier operation whereas being an
>  ARMv5 for other operations, hence a special --enable-arm-mach=pxa3xx option in
>  configure.in.

After a quick look at all architectures xenomai supports, it seems
that we can implement atomic_cmpxchg for most of them in user-space.
The only one for which I have a doubt is blackfin. Since linux for
blackfin is uclinux, can we call cli/sti from user-space ?

-- 
 Gilles


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Xenomai-core] [Patch 4/7] Define ARM atomic operations in user-space
  2008-05-05 16:33         ` [Xenomai-core] [Patch 4/7] Define ARM atomic operations in user-space Gilles Chanteperdrix
@ 2008-05-05 16:39           ` Philippe Gerum
  2008-05-05 16:44             ` Gilles Chanteperdrix
  0 siblings, 1 reply; 34+ messages in thread
From: Philippe Gerum @ 2008-05-05 16:39 UTC (permalink / raw)
  To: Gilles Chanteperdrix; +Cc: xenomai

Gilles Chanteperdrix wrote:
> On Sat, May 3, 2008 at 12:34 AM, Gilles Chanteperdrix
> <gilles.chanteperdrix@xenomai.org> wrote:
>>  The include/asm-arm/atomic.h header now defines the xnarch_memory_barrier in
>>  addition to user-space atomic operations. The pxa3xx deserves a special
>>  treatment since it uses the ARMv6 memory barrier operation whereas being an
>>  ARMv5 for other operations, hence a special --enable-arm-mach=pxa3xx option in
>>  configure.in.
> 
> After a quick look at all architectures xenomai supports, it seems
> that we can implement atomic_cmpxchg for most of them in user-space.
> The only one for which I have a doubt is blackfin. Since linux for
> blackfin is uclinux, can we call cli/sti from user-space ?
> 

Nope, these are supervisor's toys on this arch. IIRC, recent Blackfin releases
should be providing fast atomic ops from userland despite this, by using
protected jumps to kernel space for that purpose. I need to check that in the
kernel code, this was a discussion running on the Blackfin dev-list months ago.
Anyway, I would suggest to exclude Blackfin from the mutex optimization set
right now, then go back to it when we support their latest kernel.

-- 
Philippe.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Xenomai-core] [Patch 4/7] Define ARM atomic operations in user-space
  2008-05-05 16:39           ` Philippe Gerum
@ 2008-05-05 16:44             ` Gilles Chanteperdrix
  0 siblings, 0 replies; 34+ messages in thread
From: Gilles Chanteperdrix @ 2008-05-05 16:44 UTC (permalink / raw)
  To: rpm; +Cc: xenomai

On Mon, May 5, 2008 at 6:39 PM, Philippe Gerum <rpm@xenomai.org> wrote:
>
> Gilles Chanteperdrix wrote:
>  > On Sat, May 3, 2008 at 12:34 AM, Gilles Chanteperdrix
>  > <gilles.chanteperdrix@xenomai.org> wrote:
>  >>  The include/asm-arm/atomic.h header now defines the xnarch_memory_barrier in
>  >>  addition to user-space atomic operations. The pxa3xx deserves a special
>  >>  treatment since it uses the ARMv6 memory barrier operation whereas being an
>  >>  ARMv5 for other operations, hence a special --enable-arm-mach=pxa3xx option in
>  >>  configure.in.
>  >
>  > After a quick look at all architectures xenomai supports, it seems
>  > that we can implement atomic_cmpxchg for most of them in user-space.
>  > The only one for which I have a doubt is blackfin. Since linux for
>  > blackfin is uclinux, can we call cli/sti from user-space ?
>  >
>
>  Nope, these are supervisor's toys on this arch. IIRC, recent Blackfin releases
>  should be providing fast atomic ops from userland despite this, by using
>  protected jumps to kernel space for that purpose. I need to check that in the
>  kernel code, this was a discussion running on the Blackfin dev-list months ago.

It looks like the same technique as used on pre-v6 ARM.

>  Anyway, I would suggest to exclude Blackfin from the mutex optimization set
>  right now, then go back to it when we support their latest kernel.

Ok. I was dreaming about removing all the #ifdef
XNARCH_HAVE_US_ATOMIC_CMPXCHG. But it is probably better to leave it
after all. If for anything, it will help people doing ports to new
architectures gradually.

-- 
 Gilles


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Xenomai-core] [Patch 1/7] Support for non cached memory mappings
  2008-05-02 22:30 ` [Xenomai-core] [Patch 1/7] Support for non cached memory mappings Gilles Chanteperdrix
  2008-05-02 22:32   ` [Xenomai-core] [Patch 2/7] Define XNARCH_SHARED_HEAP_FLAGS Gilles Chanteperdrix
@ 2008-05-18 16:30   ` Philippe Gerum
  1 sibling, 0 replies; 34+ messages in thread
From: Philippe Gerum @ 2008-05-18 16:30 UTC (permalink / raw)
  To: Gilles Chanteperdrix; +Cc: xenomai

Gilles Chanteperdrix wrote:
> In addition to support for non cached memory mappings, this patch implements
> xnheap_init_mapped and xnheap_destroy_mapped in the !CONFIG_XENO_OPT_PERVASIVE
> case. This avoids a lot of #ifdefs for users of these functions without
> user-space support (posix skin shared memories, and the new semaphore heaps,
> heaps where is allocated the memory used for storing semaphores counters and
> mutexes owner fields).
>

Looks good. I would suggest to make xnheap_init_mapped() whine and bail out when
non-cached DMA memory is requested though.

> ---
>  include/asm-generic/wrappers.h |   12 +++++---
>  include/native/heap.h          |    1
>  include/nucleus/heap.h         |    2 +
>  ksrc/nucleus/heap.c            |   55 +++++++++++++++++++++++++++++++++++------
>  ksrc/skins/native/heap.c       |    8 +++++
>  5 files changed, 66 insertions(+), 12 deletions(-)
> 
> Index: include/native/heap.h
> ===================================================================
> --- include/native/heap.h	(revision 3738)
> +++ 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 3738)
> +++ 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 3738)
> +++ 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 3738)
> +++ 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 3738)
> +++ 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)
> @@ -1284,11 +1297,39 @@ int xnheap_destroy_mapped(xnheap_t *heap
>  	return 0;
>  }
>  
> -EXPORT_SYMBOL(xnheap_init_mapped);
> -EXPORT_SYMBOL(xnheap_destroy_mapped);
> +#else /* !CONFIG_XENO_OPT_PERVASIVE */
> +static void xnheap_free_extent(xnheap_t *heap,
> +			       void *extent, u_long size, void *cookie)
> +{
> +	xnarch_free_host_mem(extent, size);
> +}
> +
> +int xnheap_init_mapped(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 xnheap_destroy_mapped(xnheap_t *heap)
> +{
> +	xnheap_destroy(heap, &xnheap_free_extent, NULL);
> +}
>  #endif /* CONFIG_XENO_OPT_PERVASIVE */
>  
> +EXPORT_SYMBOL(xnheap_init_mapped);
> +EXPORT_SYMBOL(xnheap_destroy_mapped);
> +
>  /*@}*/
>  
>  EXPORT_SYMBOL(xnheap_alloc);
> 


-- 
Philippe.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Xenomai-core] [Patch 5/7] Define new syscalls for the system skin
  2008-05-02 22:35         ` [Xenomai-core] [Patch 5/7] Define new syscalls for the system skin Gilles Chanteperdrix
  2008-05-02 22:36           ` [Xenomai-core] [Patch 6/7] Re-implementation of mutexes, kernel-space support Gilles Chanteperdrix
@ 2008-05-18 16:40           ` Philippe Gerum
  2008-05-18 17:21             ` Gilles Chanteperdrix
  2008-05-18 18:37             ` Gilles Chanteperdrix
  1 sibling, 2 replies; 34+ messages in thread
From: Philippe Gerum @ 2008-05-18 16:40 UTC (permalink / raw)
  To: Gilles Chanteperdrix; +Cc: xenomai

Gilles Chanteperdrix wrote:
> The two syscalls defined in the posix skin now moved to the sys skin, they are
> used in user-space by include/asm-generic/bits/bind.h and the new header
> include/asm-generic/bits/current.h. The global and process-specific shared heaps
> are now part of this patch.
>

Is there any reason why the nucleus should not implement a full-fledged "RT
futex" support, instead of a toolbox to build them? I'm concerned by skins
reinventing their own wheel uselessly to get to the same point at the end of the
day; e.g. cb_lock ops seem to me fairly generic when it comes to handling
futexes, so I would move them upstream one level more.

In that respect, talking about "semaphore heaps" at nucleus level looks a bit of
a misnomer: if we mostly bring a service to map non-cacheable memory to
user-space, then we don't actually provide semaphore support.

> ---
>  include/asm-generic/bits/Makefile.am |    1
>  include/asm-generic/bits/bind.h      |  121 +++++++++++++++++++++++++++++++++++
>  include/asm-generic/bits/current.h   |   14 ++++
>  include/asm-generic/syscall.h        |    2
>  include/nucleus/Makefile.am          |    1
>  include/nucleus/sys_ppd.h            |   37 ++++++++++
>  include/posix/syscall.h              |    1
>  ksrc/nucleus/Kconfig                 |   25 +++++++
>  ksrc/nucleus/module.c                |   19 ++++-
>  ksrc/nucleus/shadow.c                |  103 ++++++++++++++++++++++++++++-
>  src/skins/posix/thread.c             |    4 +
>  11 files changed, 323 insertions(+), 5 deletions(-)
> 
> Index: include/asm-generic/syscall.h
> ===================================================================
> --- include/asm-generic/syscall.h	(revision 3738)
> +++ include/asm-generic/syscall.h	(working copy)
> @@ -30,6 +30,8 @@
>  #define __xn_sys_info       4	/* xnshadow_get_info(muxid,&info) */
>  #define __xn_sys_arch       5	/* r = xnarch_local_syscall(args) */
>  #define __xn_sys_trace      6	/* r = xntrace_xxx(...) */
> +#define __xn_sys_sem_heap   7
> +#define __xn_sys_current    8
>  
>  #define XENOMAI_LINUX_DOMAIN  0
>  #define XENOMAI_XENO_DOMAIN   1
> Index: ksrc/nucleus/Kconfig
> ===================================================================
> --- ksrc/nucleus/Kconfig	(revision 3738)
> +++ ksrc/nucleus/Kconfig	(working copy)
> @@ -158,6 +158,31 @@ config XENO_OPT_SYS_STACKPOOLSZ
>  	default 0
>  endif	
>  
> +config XENO_OPT_SEM_HEAPSZ
> +	int "Size of private semaphores heap (Kb)"
> +	default 12
> +	help
> +
> +	Xenomai implementation of user-space semaphores relies on heaps 
> +	shared between kernel and user-space. This configuration entry
> +	allow to set the size of the heap used for private semaphores.
> +
> +	Note that each semaphore will allocate 4 or 8 bytes of memory,

It would be nice to tell the folks why such difference (32/64bit arch?)

> +	so, the default of 12 Kb allows creating many semaphores.
> +
> +config XENO_OPT_GLOBAL_SEM_HEAPSZ
> +	int "Size of global semaphores heap (Kb)"
> +	default 12
> +	help
> +
> +	Xenomai implementation of user-space semaphores relies on heaps 
> +	shared between kernel and user-space. This configuration entry
> +	allow to set the size of the heap used for semaphores shared 
> +	between several processes.
> +
> +	Note that each semaphore will allocate 4 or 8 bytes of memory,
> +	so, the default of 12 Kb allows creating many semaphores.
> +
>  config XENO_OPT_STATS
>  	bool "Statistics collection"
>  	depends on PROC_FS
> Index: ksrc/nucleus/module.c
> ===================================================================
> --- ksrc/nucleus/module.c	(revision 3738)
> +++ ksrc/nucleus/module.c	(working copy)
> @@ -28,6 +28,7 @@
>  #include <nucleus/timer.h>
>  #include <nucleus/heap.h>
>  #include <nucleus/version.h>
> +#include <nucleus/sys_ppd.h>
>  #ifdef CONFIG_XENO_OPT_PIPE
>  #include <nucleus/pipe.h>
>  #endif /* CONFIG_XENO_OPT_PIPE */
> @@ -50,6 +51,8 @@ u_long xnmod_sysheap_size;
>  
>  int xeno_nucleus_status = -EINVAL;
>  
> +struct xnsys_ppd __xnsys_global_ppd;
> +
>  void xnmod_alloc_glinks(xnqueue_t *freehq)
>  {
>  	xngholder_t *sholder, *eholder;
> @@ -1156,6 +1159,14 @@ int __init __xeno_sys_init(void)
>  	if (err)
>  		goto fail;
>  
> +	err = xnheap_init_mapped(&__xnsys_global_ppd.sem_heap,
> +				 CONFIG_XENO_OPT_GLOBAL_SEM_HEAPSZ * 1024,
> +				 XNARCH_SHARED_HEAP_FLAGS ?:
> +				 (CONFIG_XENO_OPT_GLOBAL_SEM_HEAPSZ <= 128
> +				  ? GFP_USER : 0));
> +	if (err)
> +		goto cleanup_arch;
> +	
>  #ifdef __KERNEL__
>  #ifdef CONFIG_PROC_FS
>  	xnpod_init_proc();
> @@ -1167,7 +1178,7 @@ int __init __xeno_sys_init(void)
>  	err = xnpipe_mount();
>  
>  	if (err)
> -		goto cleanup_arch;
> +		goto cleanup_proc;
>  #endif /* CONFIG_XENO_OPT_PIPE */
>  
>  #ifdef CONFIG_XENO_OPT_PERVASIVE
> @@ -1207,7 +1218,7 @@ int __init __xeno_sys_init(void)
>  #ifdef CONFIG_XENO_OPT_PIPE
>  	xnpipe_umount();
>  
> -      cleanup_arch:
> +      cleanup_proc:
>  
>  #endif /* CONFIG_XENO_OPT_PIPE */
>  
> @@ -1215,6 +1226,8 @@ int __init __xeno_sys_init(void)
>  	xnpod_delete_proc();
>  #endif /* CONFIG_PROC_FS */
>  
> +      cleanup_arch:
> +
>  	xnarch_exit();
>  
>  #endif /* __KERNEL__ */
> @@ -1254,6 +1267,8 @@ void __exit __xeno_sys_exit(void)
>  #endif /* CONFIG_XENO_OPT_PIPE */
>  #endif /* __KERNEL__ */
>  
> +	xnheap_destroy_mapped(&__xnsys_global_ppd.sem_heap);
> +
>  	if (nkmsgbuf)
>  		xnarch_free_host_mem(nkmsgbuf, XNPOD_FATAL_BUFSZ);
>  
> Index: ksrc/nucleus/shadow.c
> ===================================================================
> --- ksrc/nucleus/shadow.c	(revision 3738)
> +++ ksrc/nucleus/shadow.c	(working copy)
> @@ -51,6 +51,7 @@
>  #include <nucleus/ppd.h>
>  #include <nucleus/trace.h>
>  #include <nucleus/stat.h>
> +#include <nucleus/sys_ppd.h>
>  #include <asm/xenomai/features.h>
>  #include <asm/xenomai/syscall.h>
>  #include <asm/xenomai/bits/shadow.h>
> @@ -1558,11 +1559,11 @@ static void stringify_feature_set(u_long
>  
>  static int xnshadow_sys_bind(struct pt_regs *regs)
>  {
> +	xnshadow_ppd_t *ppd = NULL, *sys_ppd = NULL;
>  	unsigned magic = __xn_reg_arg1(regs);
>  	u_long featdep = __xn_reg_arg2(regs);
>  	u_long abirev = __xn_reg_arg3(regs);
>  	u_long infarg = __xn_reg_arg4(regs);
> -	xnshadow_ppd_t *ppd = NULL;
>  	xnfeatinfo_t finfo;
>  	u_long featmis;
>  	int muxid, err;
> @@ -1636,6 +1637,36 @@ static int xnshadow_sys_bind(struct pt_r
>  	   earlier than that, do not refer to nkpod until the latter had a
>  	   chance to call xnpod_init(). */
>  
> +	xnlock_get_irqsave(&nklock, s);
> +	sys_ppd = ppd_lookup(0, current->mm);
> +	xnlock_put_irqrestore(&nklock, s);
> +
> +	if (sys_ppd)
> +		goto muxid_eventcb;
> +
> +	sys_ppd = (xnshadow_ppd_t *) muxtable[0].props->eventcb(XNSHADOW_CLIENT_ATTACH,
> +								 current);
> +
> +	if (IS_ERR(sys_ppd)) {
> +		err = PTR_ERR(sys_ppd);
> +		goto fail;
> +	}
> +
> +	if (!sys_ppd)
> +		goto muxid_eventcb;
> +
> +
> +	sys_ppd->key.muxid = 0;
> +	sys_ppd->key.mm = current->mm;
> +
> +	if (ppd_insert(sys_ppd) == -EBUSY) {
> +		/* In case of concurrent binding (which can not happen with
> +		   Xenomai libraries), detach right away the second ppd. */
> +		muxtable[0].props->eventcb(XNSHADOW_CLIENT_DETACH, sys_ppd);
> +		sys_ppd = NULL;
> +	}
> +
> +  muxid_eventcb:
>  	if (!muxtable[muxid].props->eventcb)
>  		goto eventcb_done;
>  
> @@ -1652,7 +1683,7 @@ static int xnshadow_sys_bind(struct pt_r
>  
>  	if (IS_ERR(ppd)) {
>  		err = PTR_ERR(ppd);
> -		goto fail;
> +		goto fail_destroy_sys_ppd;
>  	}
>  
>  	if (!ppd)
> @@ -1689,6 +1720,11 @@ static int xnshadow_sys_bind(struct pt_r
>  
>  		err = -ENOSYS;
>  
> +	  fail_destroy_sys_ppd:
> +		if (sys_ppd) {
> +			ppd_remove(sys_ppd);
> +			muxtable[0].props->eventcb(XNSHADOW_CLIENT_DETACH, sys_ppd);
> +		}
>  	      fail:
>  		if (!xnarch_atomic_get(&muxtable[muxid].refcnt))
>  			xnarch_atomic_dec(&muxtable[muxid].refcnt);
> @@ -1856,6 +1892,32 @@ static int xnshadow_sys_trace(struct pt_
>  	return err;
>  }
>  
> +struct heap_info {
> +	xnheap_t *addr;
> +	unsigned size;
> +};
> +
> +static int xnshadow_sys_sem_heap(struct pt_regs *regs)
> +{
> +	struct heap_info hinfo, __user *us_hinfo;
> +	unsigned global;
> +
> +	global = __xn_reg_arg2(regs);
> +	us_hinfo = (struct heap_info __user *) __xn_reg_arg1(regs);
> +	hinfo.addr = &xnsys_ppd_get(global)->sem_heap;
> +	hinfo.size = xnheap_extentsize(hinfo.addr);
> +
> +	return __xn_safe_copy_to_user(us_hinfo, &hinfo, sizeof(*us_hinfo));
> +}
> +
> +static int xnshadow_sys_current(struct pt_regs *regs)
> +{
> +	xnthread_t * __user *us_current, *cur = xnshadow_thread(current);
> +	us_current = (xnthread_t *__user *) __xn_reg_arg1(regs);
> +
> +	return __xn_safe_copy_to_user(us_current, &cur, sizeof(*us_current));
> +}
> +
>  static xnsysent_t __systab[] = {
>  	[__xn_sys_migrate] = {&xnshadow_sys_migrate, __xn_exec_current},
>  	[__xn_sys_arch] = {&xnshadow_sys_arch, __xn_exec_any},
> @@ -1864,15 +1926,50 @@ static xnsysent_t __systab[] = {
>  	[__xn_sys_completion] = {&xnshadow_sys_completion, __xn_exec_lostage},
>  	[__xn_sys_barrier] = {&xnshadow_sys_barrier, __xn_exec_lostage},
>  	[__xn_sys_trace] = {&xnshadow_sys_trace, __xn_exec_any},
> +	[__xn_sys_sem_heap] = {&xnshadow_sys_sem_heap, __xn_exec_any},
> +	[__xn_sys_current] = {&xnshadow_sys_current, __xn_exec_any},
>  };
>  
> +static void *xnshadow_sys_event(int event, void *data)
> +{
> +	struct xnsys_ppd *p;
> +	int err;
> +
> +	switch(event) {
> +	case XNSHADOW_CLIENT_ATTACH:
> +		p = (struct xnsys_ppd *) xnarch_alloc_host_mem(sizeof(*p));
> +		if (!p)
> +			return ERR_PTR(-ENOMEM);
> +
> +		err = xnheap_init_mapped(&p->sem_heap,
> +					 CONFIG_XENO_OPT_SEM_HEAPSZ * 1024,
> +					 XNARCH_SHARED_HEAP_FLAGS ?:
> +					 (CONFIG_XENO_OPT_SEM_HEAPSZ <= 128
> +					  ? GFP_USER : 0));
> +		if (err) {
> +			xnarch_free_host_mem(p, sizeof(*p));
> +			return ERR_PTR(err);
> +		}
> +
> +		return &p->ppd;
> +
> +	case XNSHADOW_CLIENT_DETACH:
> +		p = ppd2sys((xnshadow_ppd_t *) data);
> +
> +		xnheap_destroy_mapped(&p->sem_heap);
> +
> +		return NULL;
> +	}
> +
> +	return ERR_PTR(-EINVAL);
> +}
>  
>  static struct xnskin_props __props = {
>  	.name = "sys",
>  	.magic = 0x434F5245,
>  	.nrcalls = sizeof(__systab) / sizeof(__systab[0]),
>  	.systab = __systab,
> -	.eventcb = NULL,
> +	.eventcb = xnshadow_sys_event,
>  	.timebasep = NULL,
>  	.module = NULL
>  };
> Index: include/posix/syscall.h
> ===================================================================
> --- include/posix/syscall.h	(revision 3738)
> +++ 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
> Index: src/skins/posix/thread.c
> ===================================================================
> --- src/skins/posix/thread.c	(revision 3738)
> +++ src/skins/posix/thread.c	(working copy)
> @@ -24,6 +24,7 @@
>  #include <sys/types.h>
>  #include <semaphore.h>
>  #include <posix/syscall.h>
> +#include <asm-generic/bits/current.h>
>  
>  extern int __pse51_muxid;
>  
> @@ -56,6 +57,7 @@ int __wrap_pthread_setschedparam(pthread
>  
>  	if (!err && promoted) {
>  		old_sigharden_handler = signal(SIGHARDEN, &__pthread_sigharden_handler);
> +		xeno_set_current();
>  		if (policy != SCHED_OTHER)
>  			XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_XENO_DOMAIN);
>  	}
> @@ -132,6 +134,8 @@ static void *__pthread_trampoline(void *
>  	start = iargs->start;
>  	cookie = iargs->arg;
>  
> +	xeno_set_current();
> +
>  	__real_sem_post(&iargs->sync);
>  
>  	if (!err) {
> Index: include/asm-generic/bits/current.h
> ===================================================================
> --- include/asm-generic/bits/current.h	(revision 0)
> +++ include/asm-generic/bits/current.h	(revision 0)
> @@ -0,0 +1,14 @@
> +#ifndef _XENO_ASM_GENERIC_CURRENT_H
> +#define _XENO_ASM_GENERIC_CURRENT_H
> +
> +#include <pthread.h>
> +
> +extern pthread_key_t xeno_current_key;
> +
> +extern void xeno_set_current(void);
> +
> +static inline void *xeno_get_current(void)
> +{
> +	return pthread_getspecific(xeno_current_key);
> +}
> +#endif /* _XENO_ASM_GENERIC_CURRENT_H */
> Index: include/asm-generic/bits/Makefile.am
> ===================================================================
> --- include/asm-generic/bits/Makefile.am	(revision 3738)
> +++ include/asm-generic/bits/Makefile.am	(working copy)
> @@ -2,6 +2,7 @@ includesubdir = $(includedir)/asm-generi
>  
>  includesub_HEADERS = \
>  	bind.h \
> +	current.h \
>  	heap.h \
>  	intr.h \
>  	mlock_alert.h \
> Index: include/nucleus/sys_ppd.h
> ===================================================================
> --- include/nucleus/sys_ppd.h	(revision 0)
> +++ include/nucleus/sys_ppd.h	(revision 0)
> @@ -0,0 +1,37 @@
> +#ifndef _XENO_NUCLEUS_SYS_PPD_H
> +#define _XENO_NUCLEUS_SYS_PPD_H
> +
> +#include <nucleus/ppd.h>
> +#include <nucleus/heap.h>
> +
> +struct xnsys_ppd {
> +	xnshadow_ppd_t ppd;
> +	xnheap_t sem_heap;
> +
> +#define ppd2sys(addr) container_of(addr, struct xnsys_ppd, ppd)
> +};
> +
> +extern struct xnsys_ppd __xnsys_global_ppd;
> +
> +#ifdef CONFIG_XENO_OPT_PERVASIVE
> +
> +static inline struct xnsys_ppd *xnsys_ppd_get(int global)
> +{
> +	xnshadow_ppd_t *ppd;
> +
> +	if (global || !(ppd = xnshadow_ppd_get(0)))
> +		return &__xnsys_global_ppd;
> +
> +	return ppd2sys(ppd);
> +}
> +
> +#else /* !CONFIG_XENO_OPT_PERVASIVE */
> +
> +static inline struct xnsys_ppd *xnsys_ppd_get(int global)
> +{
> +	return &__xnsys_global_ppd;
> +}
> +
> +#endif /* !CONFIG_XENO_OPT_PERVASIVE */
> +
> +#endif /* _XENO_NUCLEUS_SYS_PPD_H */
> Index: include/nucleus/Makefile.am
> ===================================================================
> --- include/nucleus/Makefile.am	(revision 3738)
> +++ include/nucleus/Makefile.am	(working copy)
> @@ -20,6 +20,7 @@ includesub_HEADERS = \
>  	stat.h \
>  	synch.h \
>  	system.h \
> +	sys_ppd.h \
>  	thread.h \
>  	timebase.h \
>  	timer.h \
> Index: include/asm-generic/bits/bind.h
> ===================================================================
> --- include/asm-generic/bits/bind.h	(revision 3738)
> +++ include/asm-generic/bits/bind.h	(working copy)
> @@ -6,10 +6,89 @@
>  #include <string.h>
>  #include <errno.h>
>  #include <signal.h>
> +#include <pthread.h>
> +#include <sys/ioctl.h>
>  #include <asm/xenomai/syscall.h>
>  
> +__attribute__ ((weak))
> +pthread_key_t xeno_current_key;
> +__attribute__ ((weak))
> +pthread_once_t xeno_init_current_key_once = PTHREAD_ONCE_INIT;
> +
> +__attribute__ ((weak))
> +void xeno_set_current(void)
> +{
> +	void *kthread_cb;
> +	XENOMAI_SYSCALL1(__xn_sys_current, &kthread_cb);
> +	pthread_setspecific(xeno_current_key, kthread_cb);
> +}
> +
> +static void init_current_key(void)
> +{
> +	int err = pthread_key_create(&xeno_current_key, NULL);
> +	if (err) {
> +		fprintf(stderr, "Xenomai: error creating TSD key: %s\n",
> +			strerror(err));
> +		exit(1);
> +	}
> +}
> +
> +__attribute__ ((weak))
> +unsigned long xeno_sem_heap[2] = { 0, 0 };
> +
>  void xeno_handle_mlock_alert(int sig);
>  
> +static void *map_sem_heap(unsigned shared)
> +{
> +	struct heap_info {
> +		void *addr;
> +		unsigned size;
> +	} hinfo;
> +	int fd, err;
> +
> +	fd = open("/dev/rtheap", O_RDWR, 0);
> +	if (fd < 0) {
> +		fprintf(stderr, "Xenomai: open: %m\n");
> +		return MAP_FAILED;
> +	}
> +
> +	err = XENOMAI_SYSCALL2(__xn_sys_sem_heap, &hinfo, shared);
> +	if (err < 0) {
> +		fprintf(stderr, "Xenomai: sys_sem_heap: %m\n");
> +		return MAP_FAILED;
> +	}
> +
> +	err = ioctl(fd, 0, hinfo.addr);
> +	if (err < 0) {
> +		fprintf(stderr, "Xenomai: ioctl: %m\n");
> +		return MAP_FAILED;
> +	}
> +
> +	hinfo.addr = mmap(NULL, hinfo.size,
> +			  PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
> +	close(fd);
> +
> +	return hinfo.addr;
> +}
> +
> +static void unmap_sem_heap(unsigned long heap_addr, unsigned shared)
> +{
> +	struct heap_info {
> +		void *addr;
> +		unsigned size;
> +	} hinfo;
> +	int err;
> +
> +	err = XENOMAI_SYSCALL2(__xn_sys_sem_heap, &hinfo, shared);
> +	if (err < 0) {
> +		fprintf(stderr, "Xenomai: sys_sem_heap: %m\n");
> +		return;
> +	}
> +
> +	munmap((void *) heap_addr, hinfo.size);
> +}
> +
> +
>  static inline int
>  xeno_bind_skin(unsigned skin_magic, const char *skin, const char *module)
>  {
> @@ -62,6 +141,27 @@ xeno_bind_skin(unsigned skin_magic, cons
>  	sa.sa_flags = 0;
>  	sigaction(SIGXCPU, &sa, NULL);
>  
> +	pthread_once(&xeno_init_current_key_once, &init_current_key);
> +
> +	/* In case we forked, we need to map the new local semaphore heap */
> +	if (xeno_sem_heap[0])
> +		unmap_sem_heap(xeno_sem_heap[0], 0);
> +	xeno_sem_heap[0] = (unsigned long) map_sem_heap(0);
> +	if (xeno_sem_heap[0] == (unsigned long) MAP_FAILED) {
> +		perror("Xenomai: mmap(local sem heap)");
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	/* Even if we forked the global semaphore heap did not change, no need
> +	  to map it anew */
> +	if (!xeno_sem_heap[1]) {
> +		xeno_sem_heap[1] = (unsigned long) map_sem_heap(1);
> +		if (xeno_sem_heap[1] == (unsigned long) MAP_FAILED) {
> +			perror("Xenomai: mmap(global sem heap)");
> +			exit(EXIT_FAILURE);
> +		}
> +	}
> +
>  	return muxid;
>  }
>  
> @@ -105,6 +205,27 @@ xeno_bind_skin_opt(unsigned skin_magic, 
>  	xeno_arch_features_check();
>  #endif /* xeno_arch_features_check */
>  
> +	pthread_once(&xeno_init_current_key_once, &init_current_key);
> +
> +	/* In case we forked, we need to map the new local semaphore heap */
> +	if (xeno_sem_heap[0])
> +		unmap_sem_heap(xeno_sem_heap[0], 0);
> +	xeno_sem_heap[0] = (unsigned long) map_sem_heap(0);
> +	if (xeno_sem_heap[0] == (unsigned long) MAP_FAILED) {
> +		perror("Xenomai: mmap(local sem heap)");
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	/* Even if we forked the global semaphore heap did not change, no need
> +	  to map it anew */
> +	if (!xeno_sem_heap[1]) {
> +		xeno_sem_heap[1] = (unsigned long) map_sem_heap(1);
> +		if (xeno_sem_heap[1] == (unsigned long) MAP_FAILED) {
> +			perror("Xenomai: mmap(global sem heap)");
> +			exit(EXIT_FAILURE);
> +		}
> +	}
> +
>  	return muxid;
>  }
>  
> 


-- 
Philippe.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Xenomai-core] [Patch 7/7] Re-implementation of mutexes, user-space support.
  2008-05-02 22:38             ` [Xenomai-core] [Patch 7/7] Re-implementation of mutexes, user-space support Gilles Chanteperdrix
@ 2008-05-18 16:42               ` Philippe Gerum
  2008-05-18 17:05                 ` Gilles Chanteperdrix
  0 siblings, 1 reply; 34+ messages in thread
From: Philippe Gerum @ 2008-05-18 16:42 UTC (permalink / raw)
  To: Gilles Chanteperdrix; +Cc: xenomai

Gilles Chanteperdrix wrote:
> Since binding of the semaphore heaps is now made by xeno_skin_bind, there is
> much less modifications in src/skins/posix/init.c. However, I had to do
> something really ugly: since binding the semaphore heaps by xeno_skin_bind
> requires calls to open, ioctl, mmap, close and munmap, I redefined these symbols
> to be the __real_ variants before including bind.h.

Is there any upside to do this instead of simply calling the __real_*
placeholders, since we do already provide weak wrappers for those when the
linker's wrapping magic is not invoked?

> Another solution would have been to use __open (which would not have been
> wrapped) instead of open in bind.h and rely on the fact that the posix skin
> wrapped syscalls fall back to the __real variants when called for non real-time
> file descriptors. I do not know however, if this solution would have been
> portable. In particular, does uclibc also defines __open ?
> 
> ---
>  Makefile.am |    2
>  cond.c      |   21 ++++
>  init.c      |   26 +++++
>  mutex.c     |  265 ++++++++++++++++++++++++++++++++++++++++++++++++++++++------
>  4 files changed, 287 insertions(+), 27 deletions(-)
> 
> Index: src/skins/posix/init.c
> ===================================================================
> --- src/skins/posix/init.c	(revision 3738)
> +++ src/skins/posix/init.c	(working copy)
> @@ -26,9 +26,23 @@
>  #include <posix/posix.h>
>  #include <posix/syscall.h>
>  #include <rtdm/syscall.h>
> -#include <asm-generic/bits/bind.h>
>  #include <asm-generic/bits/mlock_alert.h>
>  
> +/* asm-generic/bits/bind.h uses the following functions, so we redefine them to
> +   be the __real variants */
> +#undef open
> +#undef ioctl
> +#undef mmap
> +#undef close
> +#undef munmap
> +#define open __real_open
> +#define ioctl __real_ioctl
> +#define mmap __real_mmap
> +#define close __real_close
> +#define munmap __real_munmap
> +
> +#include <asm-generic/bits/bind.h>
> +
>  int __pse51_muxid = -1;
>  int __rtdm_muxid = -1;
>  int __rtdm_fd_start = INT_MAX;
> @@ -91,5 +105,15 @@ void __init_posix_interface(void)
>  			exit(EXIT_FAILURE);
>  		}
>  		fork_handler_registered = 1;
> +
> +		if (sizeof(struct __shadow_mutex) > sizeof(pthread_mutex_t)) {
> +			fprintf(stderr, "sizeof(pthread_mutex_t): %d,"
> +				" sizeof(shadow_mutex): %d\n",
> +				sizeof(pthread_mutex_t),
> +				sizeof(struct __shadow_mutex));
> +			exit(EXIT_FAILURE);
> +		}
> +
>  	}
>  }
> +
> Index: src/skins/posix/mutex.c
> ===================================================================
> --- src/skins/posix/mutex.c	(revision 3738)
> +++ src/skins/posix/mutex.c	(working copy)
> @@ -17,10 +17,15 @@
>   */
>  
>  #include <errno.h>
> -#include <posix/syscall.h>
>  #include <pthread.h>
> +#include <posix/syscall.h>
> +#include <posix/cb_lock.h>
> +#include <asm-generic/bits/current.h>
> +
> +#define PSE51_MUTEX_MAGIC (0x86860303)
>  
>  extern int __pse51_muxid;
> +extern unsigned long xeno_sem_heap[2];
>  
>  int __wrap_pthread_mutexattr_init(pthread_mutexattr_t *attr)
>  {
> @@ -73,66 +78,280 @@ 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 xnarch_atomic_intptr_t *get_ownerp(struct __shadow_mutex *shadow)
> +{
> +	if (likely(!shadow->attr.pshared))
> +		return shadow->owner;
> +	
> +	return (xnarch_atomic_intptr_t *) (xeno_sem_heap[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 = (xnarch_atomic_intptr_t *)
> +			(xeno_sem_heap[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;
> +
> +	if (unlikely(cb_try_write_lock(&shadow->lock, s)))
> +		return EINVAL;
>  
> -	return -XENOMAI_SKINCALL1(__pse51_muxid,
> -				  __pse51_mutex_destroy, &_mutex->shadow_mutex);
> +	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 = xeno_get_current();
> +	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 = xnarch_atomic_intptr_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 = xeno_get_current();
> +	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 = xnarch_atomic_intptr_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 = xeno_get_current();
> +	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 = xnarch_atomic_intptr_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 -XENOMAI_SKINCALL1(__pse51_muxid,
> -				  __pse51_mutex_trylock, &_mutex->shadow_mutex);
> +	return -err;
> +
> +#else /* !XNARCH_HAVE_US_ATOMIC_CMPXCHG */
> +
> +	return -XENOMAI_SKINCALL1(__pse51_muxid, __pse51_mutex_trylock, shadow);
> +
> +#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
> +	xnarch_atomic_intptr_t *ownerp;
> +	xnthread_t *cur;
> +	int err = 0;
> +
> +	cur = xeno_get_current();
> +	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(xnarch_atomic_intptr_get(ownerp)) != cur)) {
> +		err = -EPERM;
> +		goto out_err;
> +	}
> +
> +	err = 0;
> +	if (shadow->lockcnt > 1) {
> +		--shadow->lockcnt;
> +		goto out;
> +	}
> +
> +	if (likely(xnarch_atomic_intptr_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 3738)
> +++ 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 3738)
> +++ 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 = \
> 
> 


-- 
Philippe.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Xenomai-core] [Patch 7/7] Re-implementation of mutexes, user-space support.
  2008-05-18 16:42               ` Philippe Gerum
@ 2008-05-18 17:05                 ` Gilles Chanteperdrix
  2008-05-18 17:29                   ` Jan Kiszka
  0 siblings, 1 reply; 34+ messages in thread
From: Gilles Chanteperdrix @ 2008-05-18 17:05 UTC (permalink / raw)
  To: rpm; +Cc: xenomai

Philippe Gerum wrote:
 > Gilles Chanteperdrix wrote:
 > > Since binding of the semaphore heaps is now made by xeno_skin_bind, there is
 > > much less modifications in src/skins/posix/init.c. However, I had to do
 > > something really ugly: since binding the semaphore heaps by xeno_skin_bind
 > > requires calls to open, ioctl, mmap, close and munmap, I redefined these symbols
 > > to be the __real_ variants before including bind.h.
 > 
 > Is there any upside to do this instead of simply calling the __real_*
 > placeholders, since we do already provide weak wrappers for those when the
 > linker's wrapping magic is not invoked?

The point is that the wrappers and linker magic only take place for
POSIX skins. Other skins have to call the genuine open, ioctl, mmap,
close and munmap services.


-- 


					    Gilles.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Xenomai-core] [Patch 5/7] Define new syscalls for the system skin
  2008-05-18 16:40           ` [Xenomai-core] [Patch 5/7] Define new syscalls for the system skin Philippe Gerum
@ 2008-05-18 17:21             ` Gilles Chanteperdrix
  2008-05-18 18:37             ` Gilles Chanteperdrix
  1 sibling, 0 replies; 34+ messages in thread
From: Gilles Chanteperdrix @ 2008-05-18 17:21 UTC (permalink / raw)
  To: rpm; +Cc: xenomai

Philippe Gerum wrote:
 > Gilles Chanteperdrix wrote:
 > > The two syscalls defined in the posix skin now moved to the sys skin, they are
 > > used in user-space by include/asm-generic/bits/bind.h and the new header
 > > include/asm-generic/bits/current.h. The global and process-specific shared heaps
 > > are now part of this patch.
 > >
 > 
 > Is there any reason why the nucleus should not implement a full-fledged "RT
 > futex" support, instead of a toolbox to build them? I'm concerned by skins
 > reinventing their own wheel uselessly to get to the same point at the end of the
 > day; e.g. cb_lock ops seem to me fairly generic when it comes to handling
 > futexes, so I would move them upstream one level more.
 > 

First of all, because I do not know much what a futex is, but from my
point of view it has very much to do with making threads wait in
kernel-space for a user-space change. By using kernel/user shared heaps,
we seem to be far from these considerations.

Now for the implementation of user-space mutexes per-se, apart from the
xnarch_atomic_ operations, I think every skin will have its own
tradeoffs. For instance, the "cb_lock" thing is itself a tradeoff, it
sacrifices a bit of performance by making a mutex lock operation use
three atomic operations instead of only one for the sake of safely
getting a correct value of the mapped memory pointer. Besides, it works
well with the posix skin, because the posix specification allows
pthread_mutex_destroy to return an error if a mutex is currently in
use. Currently, the native skin mutex destruction operation is
successful even if the mutex is currently in use (the owner does not get
any notification, simply its unlock operation will fail, and waiters
receive an -EIDRM error). So, I would guess the native skin will have to
use something else than cb_lock.

 > In that respect, talking about "semaphore heaps" at nucleus level looks a bit of
 > a misnomer: if we mostly bring a service to map non-cacheable memory to
 > user-space, then we don't actually provide semaphore support.

It was only a name making clear to the user what the heaps will be used
for... I imagine people having to configure yet another heap size in
kernel configuration and wondering what this heap will be used for.

-- 


					    Gilles.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Xenomai-core] [Patch 7/7] Re-implementation of mutexes, user-space support.
  2008-05-18 17:05                 ` Gilles Chanteperdrix
@ 2008-05-18 17:29                   ` Jan Kiszka
  2008-05-18 17:41                     ` Gilles Chanteperdrix
  0 siblings, 1 reply; 34+ messages in thread
From: Jan Kiszka @ 2008-05-18 17:29 UTC (permalink / raw)
  To: Gilles Chanteperdrix; +Cc: xenomai

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

Gilles Chanteperdrix wrote:
> Philippe Gerum wrote:
>  > Gilles Chanteperdrix wrote:
>  > > Since binding of the semaphore heaps is now made by xeno_skin_bind, there is
>  > > much less modifications in src/skins/posix/init.c. However, I had to do
>  > > something really ugly: since binding the semaphore heaps by xeno_skin_bind
>  > > requires calls to open, ioctl, mmap, close and munmap, I redefined these symbols
>  > > to be the __real_ variants before including bind.h.
>  > 
>  > Is there any upside to do this instead of simply calling the __real_*
>  > placeholders, since we do already provide weak wrappers for those when the
>  > linker's wrapping magic is not invoked?
> 
> The point is that the wrappers and linker magic only take place for
> POSIX skins. Other skins have to call the genuine open, ioctl, mmap,
> close and munmap services.

What about controlling the desired prefix for generic functions in
bind.h via some #define that the caller has to/can set before including
the header:

#define POSIX_PREFIX	__real_
#include <asm-generic/bits/bind.h>

and there we would have:

POSIX_PREFIX##open(...);

Looks a bit cleaner to me.

Jan


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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Xenomai-core] [Patch 7/7] Re-implementation of mutexes, user-space support.
  2008-05-18 17:29                   ` Jan Kiszka
@ 2008-05-18 17:41                     ` Gilles Chanteperdrix
  2008-05-18 17:52                       ` Jan Kiszka
  0 siblings, 1 reply; 34+ messages in thread
From: Gilles Chanteperdrix @ 2008-05-18 17:41 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: xenomai

Jan Kiszka wrote:
 > Gilles Chanteperdrix wrote:
 > > Philippe Gerum wrote:
 > >  > Gilles Chanteperdrix wrote:
 > >  > > Since binding of the semaphore heaps is now made by xeno_skin_bind, there is
 > >  > > much less modifications in src/skins/posix/init.c. However, I had to do
 > >  > > something really ugly: since binding the semaphore heaps by xeno_skin_bind
 > >  > > requires calls to open, ioctl, mmap, close and munmap, I redefined these symbols
 > >  > > to be the __real_ variants before including bind.h.
 > >  > 
 > >  > Is there any upside to do this instead of simply calling the __real_*
 > >  > placeholders, since we do already provide weak wrappers for those when the
 > >  > linker's wrapping magic is not invoked?
 > > 
 > > The point is that the wrappers and linker magic only take place for
 > > POSIX skins. Other skins have to call the genuine open, ioctl, mmap,
 > > close and munmap services.
 > 
 > What about controlling the desired prefix for generic functions in
 > bind.h via some #define that the caller has to/can set before including
 > the header:
 > 
 > #define POSIX_PREFIX	__real_
 > #include <asm-generic/bits/bind.h>
 > 
 > and there we would have:
 > 
 > POSIX_PREFIX##open(...);
 > 
 > Looks a bit cleaner to me.

Well, in this case we end up cluttering the code with the POSIX_PREFIX
macro, even in the non posix case where no prefix is needed.

-- 


					    Gilles.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Xenomai-core] [Patch 7/7] Re-implementation of mutexes, user-space support.
  2008-05-18 17:41                     ` Gilles Chanteperdrix
@ 2008-05-18 17:52                       ` Jan Kiszka
  2008-05-18 18:09                         ` Gilles Chanteperdrix
  0 siblings, 1 reply; 34+ messages in thread
From: Jan Kiszka @ 2008-05-18 17:52 UTC (permalink / raw)
  To: Gilles Chanteperdrix; +Cc: xenomai

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

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>  > Gilles Chanteperdrix wrote:
>  > > Philippe Gerum wrote:
>  > >  > Gilles Chanteperdrix wrote:
>  > >  > > Since binding of the semaphore heaps is now made by xeno_skin_bind, there is
>  > >  > > much less modifications in src/skins/posix/init.c. However, I had to do
>  > >  > > something really ugly: since binding the semaphore heaps by xeno_skin_bind
>  > >  > > requires calls to open, ioctl, mmap, close and munmap, I redefined these symbols
>  > >  > > to be the __real_ variants before including bind.h.
>  > >  > 
>  > >  > Is there any upside to do this instead of simply calling the __real_*
>  > >  > placeholders, since we do already provide weak wrappers for those when the
>  > >  > linker's wrapping magic is not invoked?
>  > > 
>  > > The point is that the wrappers and linker magic only take place for
>  > > POSIX skins. Other skins have to call the genuine open, ioctl, mmap,
>  > > close and munmap services.
>  > 
>  > What about controlling the desired prefix for generic functions in
>  > bind.h via some #define that the caller has to/can set before including
>  > the header:
>  > 
>  > #define POSIX_PREFIX	__real_
>  > #include <asm-generic/bits/bind.h>
>  > 
>  > and there we would have:
>  > 
>  > POSIX_PREFIX##open(...);
>  > 
>  > Looks a bit cleaner to me.
> 
> Well, in this case we end up cluttering the code with the POSIX_PREFIX
> macro, even in the non posix case where no prefix is needed.

Yes, but there are only few spots. The advantage of this strategy is
that it is explicit in-place (ie. inside bind.h). That avoids potential
collateral damage in the future when other services are added to that
helper which shall not be wrapped.

Jan


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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Xenomai-core] [Patch 7/7] Re-implementation of mutexes, user-space support.
  2008-05-18 17:52                       ` Jan Kiszka
@ 2008-05-18 18:09                         ` Gilles Chanteperdrix
  2008-05-18 18:56                           ` Philippe Gerum
  0 siblings, 1 reply; 34+ messages in thread
From: Gilles Chanteperdrix @ 2008-05-18 18:09 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: xenomai

Jan Kiszka wrote:
 > Gilles Chanteperdrix wrote:
 > > Jan Kiszka wrote:
 > >  > Gilles Chanteperdrix wrote:
 > >  > > Philippe Gerum wrote:
 > >  > >  > Gilles Chanteperdrix wrote:
 > >  > >  > > Since binding of the semaphore heaps is now made by xeno_skin_bind, there is
 > >  > >  > > much less modifications in src/skins/posix/init.c. However, I had to do
 > >  > >  > > something really ugly: since binding the semaphore heaps by xeno_skin_bind
 > >  > >  > > requires calls to open, ioctl, mmap, close and munmap, I redefined these symbols
 > >  > >  > > to be the __real_ variants before including bind.h.
 > >  > >  > 
 > >  > >  > Is there any upside to do this instead of simply calling the __real_*
 > >  > >  > placeholders, since we do already provide weak wrappers for those when the
 > >  > >  > linker's wrapping magic is not invoked?
 > >  > > 
 > >  > > The point is that the wrappers and linker magic only take place for
 > >  > > POSIX skins. Other skins have to call the genuine open, ioctl, mmap,
 > >  > > close and munmap services.
 > >  > 
 > >  > What about controlling the desired prefix for generic functions in
 > >  > bind.h via some #define that the caller has to/can set before including
 > >  > the header:
 > >  > 
 > >  > #define POSIX_PREFIX	__real_
 > >  > #include <asm-generic/bits/bind.h>
 > >  > 
 > >  > and there we would have:
 > >  > 
 > >  > POSIX_PREFIX##open(...);
 > >  > 
 > >  > Looks a bit cleaner to me.
 > > 
 > > Well, in this case we end up cluttering the code with the POSIX_PREFIX
 > > macro, even in the non posix case where no prefix is needed.
 > 
 > Yes, but there are only few spots. The advantage of this strategy is
 > that it is explicit in-place (ie. inside bind.h). That avoids potential
 > collateral damage in the future when other services are added to that
 > helper which shall not be wrapped.

Actually, the only important call is open. Since once the file
descriptor has been created with __real_open, all syscall wrappers will
automatically fall back to the __real syscall variants.

-- 


					    Gilles.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Xenomai-core] [Patch 5/7] Define new syscalls for the system skin
  2008-05-18 16:40           ` [Xenomai-core] [Patch 5/7] Define new syscalls for the system skin Philippe Gerum
  2008-05-18 17:21             ` Gilles Chanteperdrix
@ 2008-05-18 18:37             ` Gilles Chanteperdrix
  2008-05-18 19:10               ` Philippe Gerum
  1 sibling, 1 reply; 34+ messages in thread
From: Gilles Chanteperdrix @ 2008-05-18 18:37 UTC (permalink / raw)
  To: rpm; +Cc: xenomai

Philippe Gerum wrote:
 > Gilles Chanteperdrix wrote:
 > > The two syscalls defined in the posix skin now moved to the sys skin, they are
 > > used in user-space by include/asm-generic/bits/bind.h and the new header
 > > include/asm-generic/bits/current.h. The global and process-specific shared heaps
 > > are now part of this patch.
 > >
 > 
 > Is there any reason why the nucleus should not implement a full-fledged "RT
 > futex" support, instead of a toolbox to build them? I'm concerned by skins
 > reinventing their own wheel uselessly to get to the same point at the end of the
 > day; e.g. cb_lock ops seem to me fairly generic when it comes to handling
 > futexes, so I would move them upstream one level more.
 > 
 > In that respect, talking about "semaphore heaps" at nucleus level looks a bit of
 > a misnomer: if we mostly bring a service to map non-cacheable memory to
 > user-space, then we don't actually provide semaphore support.

If I understand correctly, a futex is, in xenomai terms, a way to
associate a user-space address, with an xnsynch object. I feel this
would complicate things: currently, the way I implemented user-space
mutexes for the posix skin kept the old association between the
user-space mutex, and its kernel-space companion, also used by
kernel-space operations.

-- 


					    Gilles.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Xenomai-core] [Patch 7/7] Re-implementation of mutexes, user-space support.
  2008-05-18 18:09                         ` Gilles Chanteperdrix
@ 2008-05-18 18:56                           ` Philippe Gerum
  2008-05-19 22:49                             ` Gilles Chanteperdrix
  0 siblings, 1 reply; 34+ messages in thread
From: Philippe Gerum @ 2008-05-18 18:56 UTC (permalink / raw)
  To: Gilles Chanteperdrix; +Cc: Jan Kiszka, xenomai

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>  > Gilles Chanteperdrix wrote:
>  > > Jan Kiszka wrote:
>  > >  > Gilles Chanteperdrix wrote:
>  > >  > > Philippe Gerum wrote:
>  > >  > >  > Gilles Chanteperdrix wrote:
>  > >  > >  > > Since binding of the semaphore heaps is now made by xeno_skin_bind, there is
>  > >  > >  > > much less modifications in src/skins/posix/init.c. However, I had to do
>  > >  > >  > > something really ugly: since binding the semaphore heaps by xeno_skin_bind
>  > >  > >  > > requires calls to open, ioctl, mmap, close and munmap, I redefined these symbols
>  > >  > >  > > to be the __real_ variants before including bind.h.
>  > >  > >  > 
>  > >  > >  > Is there any upside to do this instead of simply calling the __real_*
>  > >  > >  > placeholders, since we do already provide weak wrappers for those when the
>  > >  > >  > linker's wrapping magic is not invoked?
>  > >  > > 
>  > >  > > The point is that the wrappers and linker magic only take place for
>  > >  > > POSIX skins. Other skins have to call the genuine open, ioctl, mmap,
>  > >  > > close and munmap services.
>  > >  > 
>  > >  > What about controlling the desired prefix for generic functions in
>  > >  > bind.h via some #define that the caller has to/can set before including
>  > >  > the header:
>  > >  > 
>  > >  > #define POSIX_PREFIX	__real_
>  > >  > #include <asm-generic/bits/bind.h>
>  > >  > 
>  > >  > and there we would have:
>  > >  > 
>  > >  > POSIX_PREFIX##open(...);
>  > >  > 
>  > >  > Looks a bit cleaner to me.
>  > > 
>  > > Well, in this case we end up cluttering the code with the POSIX_PREFIX
>  > > macro, even in the non posix case where no prefix is needed.
>  > 
>  > Yes, but there are only few spots. The advantage of this strategy is
>  > that it is explicit in-place (ie. inside bind.h). That avoids potential
>  > collateral damage in the future when other services are added to that
>  > helper which shall not be wrapped.
> 
> Actually, the only important call is open. Since once the file
> descriptor has been created with __real_open, all syscall wrappers will
> automatically fall back to the __real syscall variants.
> 

Then let's just provide __real_open() as a weak symbol in all libs; that's less
error-prone that fiddling with the preprocessor.

-- 
Philippe.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Xenomai-core] [Patch 5/7] Define new syscalls for the system skin
  2008-05-18 18:37             ` Gilles Chanteperdrix
@ 2008-05-18 19:10               ` Philippe Gerum
  2008-05-18 19:38                 ` Gilles Chanteperdrix
  0 siblings, 1 reply; 34+ messages in thread
From: Philippe Gerum @ 2008-05-18 19:10 UTC (permalink / raw)
  To: Gilles Chanteperdrix; +Cc: xenomai

Gilles Chanteperdrix wrote:
> Philippe Gerum wrote:
>  > Gilles Chanteperdrix wrote:
>  > > The two syscalls defined in the posix skin now moved to the sys skin, they are
>  > > used in user-space by include/asm-generic/bits/bind.h and the new header
>  > > include/asm-generic/bits/current.h. The global and process-specific shared heaps
>  > > are now part of this patch.
>  > >
>  > 
>  > Is there any reason why the nucleus should not implement a full-fledged "RT
>  > futex" support, instead of a toolbox to build them? I'm concerned by skins
>  > reinventing their own wheel uselessly to get to the same point at the end of the
>  > day; e.g. cb_lock ops seem to me fairly generic when it comes to handling
>  > futexes, so I would move them upstream one level more.
>  > 
>  > In that respect, talking about "semaphore heaps" at nucleus level looks a bit of
>  > a misnomer: if we mostly bring a service to map non-cacheable memory to
>  > user-space, then we don't actually provide semaphore support.
> 
> If I understand correctly, a futex is, in xenomai terms, a way to
> associate a user-space address, with an xnsynch object.

I would specialize it more actually so that it really resembles the vanilla
futex support, i.e. a basic object implementing the required operations to
provide mutually exclusive access, working on a pinned memory area shared
between kernel and userland. AFAICS, the current patchset implements the pinned
memory support in the nucleus, but not the operations, which remain a per-skin
issue.

 I feel this
> would complicate things: currently, the way I implemented user-space
> mutexes for the posix skin kept the old association between the
> user-space mutex, and its kernel-space companion, also used by
> kernel-space operations.
> 

My concern boils down to: how much of the POSIX implementation, beyond the
cb_lock stuff, would have to be duplicated to get the same support ported to,
say the VxWorks semM services?

-- 
Philippe.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Xenomai-core] [Patch 5/7] Define new syscalls for the system skin
  2008-05-18 19:10               ` Philippe Gerum
@ 2008-05-18 19:38                 ` Gilles Chanteperdrix
  2008-05-18 21:52                   ` Philippe Gerum
  0 siblings, 1 reply; 34+ messages in thread
From: Gilles Chanteperdrix @ 2008-05-18 19:38 UTC (permalink / raw)
  To: rpm; +Cc: xenomai

Philippe Gerum wrote:
 > Gilles Chanteperdrix wrote:
 > > Philippe Gerum wrote:
 > >  > Gilles Chanteperdrix wrote:
 > >  > > The two syscalls defined in the posix skin now moved to the sys skin, they are
 > >  > > used in user-space by include/asm-generic/bits/bind.h and the new header
 > >  > > include/asm-generic/bits/current.h. The global and process-specific shared heaps
 > >  > > are now part of this patch.
 > >  > >
 > >  > 
 > >  > Is there any reason why the nucleus should not implement a full-fledged "RT
 > >  > futex" support, instead of a toolbox to build them? I'm concerned by skins
 > >  > reinventing their own wheel uselessly to get to the same point at the end of the
 > >  > day; e.g. cb_lock ops seem to me fairly generic when it comes to handling
 > >  > futexes, so I would move them upstream one level more.
 > >  > 
 > >  > In that respect, talking about "semaphore heaps" at nucleus level looks a bit of
 > >  > a misnomer: if we mostly bring a service to map non-cacheable memory to
 > >  > user-space, then we don't actually provide semaphore support.
 > > 
 > > If I understand correctly, a futex is, in xenomai terms, a way to
 > > associate a user-space address, with an xnsynch object.
 > 
 > I would specialize it more actually so that it really resembles the vanilla
 > futex support, i.e. a basic object implementing the required operations to
 > provide mutually exclusive access, working on a pinned memory area shared
 > between kernel and userland. AFAICS, the current patchset implements the pinned
 > memory support in the nucleus, but not the operations, which remain a per-skin
 > issue.

As far as I understood, the user-space atomic operations, used to
acquire a free mutex in user-space, are not part of the futex API. In
our case, we are using xnarch_atomic_* operations to implement portably
this user-space locking stuff. I think that even setting the bit saying
that the mutex is currently owned is done in pthread_mutexes
implementation, not in the Futex API. Now, what remains is
sys_futex(FUTEX_WAIT) and sys_futex(FUTEX_WAKE), this terribly looks like
xnsync_sleep_on and xnsynch_wakeup_one_sleeper.

 > 
 >  I feel this
 > > would complicate things: currently, the way I implemented user-space
 > > mutexes for the posix skin kept the old association between the
 > > user-space mutex, and its kernel-space companion, also used by
 > > kernel-space operations.
 > > 
 > 
 > My concern boils down to: how much of the POSIX implementation, beyond the
 > cb_lock stuff, would have to be duplicated to get the same support ported to,
 > say the VxWorks semM services?

The initialization code, and the additional calls to
xnarch_atomic_cmpxchg in user-space. If xnarch_atomic_cmpxchg fails we
call kernel-space, which is mostly unchanged.

Because of the cb_lock stuff, I also needed to implement the
kernel-space syscalls in two versions: one if user-space has
xnarch_atomic_cmpxchg and could lock the mutex control block, the other
if the mutex control block needs to be locked by kernel-space.

-- 


					    Gilles.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Xenomai-core] [Patch 5/7] Define new syscalls for the system skin
  2008-05-18 19:38                 ` Gilles Chanteperdrix
@ 2008-05-18 21:52                   ` Philippe Gerum
  2008-05-18 22:52                     ` Gilles Chanteperdrix
  0 siblings, 1 reply; 34+ messages in thread
From: Philippe Gerum @ 2008-05-18 21:52 UTC (permalink / raw)
  To: Gilles Chanteperdrix; +Cc: xenomai

Gilles Chanteperdrix wrote:
> Philippe Gerum wrote:
>  > Gilles Chanteperdrix wrote:
>  > > Philippe Gerum wrote:
>  > >  > Gilles Chanteperdrix wrote:
>  > >  > > The two syscalls defined in the posix skin now moved to the sys skin, they are
>  > >  > > used in user-space by include/asm-generic/bits/bind.h and the new header
>  > >  > > include/asm-generic/bits/current.h. The global and process-specific shared heaps
>  > >  > > are now part of this patch.
>  > >  > >
>  > >  > 
>  > >  > Is there any reason why the nucleus should not implement a full-fledged "RT
>  > >  > futex" support, instead of a toolbox to build them? I'm concerned by skins
>  > >  > reinventing their own wheel uselessly to get to the same point at the end of the
>  > >  > day; e.g. cb_lock ops seem to me fairly generic when it comes to handling
>  > >  > futexes, so I would move them upstream one level more.
>  > >  > 
>  > >  > In that respect, talking about "semaphore heaps" at nucleus level looks a bit of
>  > >  > a misnomer: if we mostly bring a service to map non-cacheable memory to
>  > >  > user-space, then we don't actually provide semaphore support.
>  > > 
>  > > If I understand correctly, a futex is, in xenomai terms, a way to
>  > > associate a user-space address, with an xnsynch object.
>  > 
>  > I would specialize it more actually so that it really resembles the vanilla
>  > futex support, i.e. a basic object implementing the required operations to
>  > provide mutually exclusive access, working on a pinned memory area shared
>  > between kernel and userland. AFAICS, the current patchset implements the pinned
>  > memory support in the nucleus, but not the operations, which remain a per-skin
>  > issue.
> 
> As far as I understood, the user-space atomic operations, used to
> acquire a free mutex in user-space, are not part of the futex API. In
> our case, we are using xnarch_atomic_* operations to implement portably
> this user-space locking stuff. I think that even setting the bit saying
> that the mutex is currently owned is done in pthread_mutexes
> implementation, not in the Futex API.

I would fully agree if the futex API did not define PI-based ops, which are
needed for proper real-time operations from userland. You will certainly agree
that PI implies that some kind of ownership exists; and because there can't be
more than a single owner in that case, you end up with an object that can't be
held by more than a single task. So you do have a mutex in disguise, whatever
the way to keep its state is (a bit, an integer, whatever). So there is stronger
semantics attached to that API than to simply manage an event notification scheme.

 Now, what remains is
> sys_futex(FUTEX_WAIT) and sys_futex(FUTEX_WAKE), this terribly looks like
> xnsync_sleep_on and xnsynch_wakeup_one_sleeper.
> 

Yes, here again I partially agree, except for a significant issue: xnsynch is a
stateless object (that's why we can use it for different syncobjs which are
totally unrelated in their semantics - mutex, queue, region, counting sems,
whatever). I was just wondering if we could make the *tex thingy a bit more
stateful to ease the job for the skins, just in case we would use it for mutexes
only. I have no immediate answer to this question, just asking -- this is my
contribution as a senior member of the peanut gallery.

>  > 
>  >  I feel this
>  > > would complicate things: currently, the way I implemented user-space
>  > > mutexes for the posix skin kept the old association between the
>  > > user-space mutex, and its kernel-space companion, also used by
>  > > kernel-space operations.
>  > > 
>  > 
>  > My concern boils down to: how much of the POSIX implementation, beyond the
>  > cb_lock stuff, would have to be duplicated to get the same support ported to,
>  > say the VxWorks semM services?
> 
> The initialization code, and the additional calls to
> xnarch_atomic_cmpxchg in user-space. If xnarch_atomic_cmpxchg fails we
> call kernel-space, which is mostly unchanged.
> 
> Because of the cb_lock stuff, I also needed to implement the
> kernel-space syscalls in two versions: one if user-space has
> xnarch_atomic_cmpxchg and could lock the mutex control block, the other
> if the mutex control block needs to be locked by kernel-space.
> 

This is the part my laziness would very much like to factor as much as possible.
If ever possible.

-- 
Philippe.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Xenomai-core] [Patch 5/7] Define new syscalls for the system skin
  2008-05-18 21:52                   ` Philippe Gerum
@ 2008-05-18 22:52                     ` Gilles Chanteperdrix
  2008-05-19 12:56                       ` Philippe Gerum
  0 siblings, 1 reply; 34+ messages in thread
From: Gilles Chanteperdrix @ 2008-05-18 22:52 UTC (permalink / raw)
  To: rpm; +Cc: xenomai

Philippe Gerum wrote:
 > Gilles Chanteperdrix wrote:
 > > As far as I understood, the user-space atomic operations, used to
 > > acquire a free mutex in user-space, are not part of the futex API. In
 > > our case, we are using xnarch_atomic_* operations to implement portably
 > > this user-space locking stuff. I think that even setting the bit saying
 > > that the mutex is currently owned is done in pthread_mutexes
 > > implementation, not in the Futex API.
 > 
 > I would fully agree if the futex API did not define PI-based ops, which are
 > needed for proper real-time operations from userland. You will certainly agree
 > that PI implies that some kind of ownership exists; and because there can't be
 > more than a single owner in that case, you end up with an object that can't be
 > held by more than a single task. So you do have a mutex in disguise, whatever
 > the way to keep its state is (a bit, an integer, whatever). So there is stronger
 > semantics attached to that API than to simply manage an event notification scheme.
 > 
 >  Now, what remains is
 > > sys_futex(FUTEX_WAIT) and sys_futex(FUTEX_WAKE), this terribly looks like
 > > xnsync_sleep_on and xnsynch_wakeup_one_sleeper.
 > > 
 > 
 > Yes, here again I partially agree, except for a significant issue: xnsynch is a
 > stateless object (that's why we can use it for different syncobjs which are
 > totally unrelated in their semantics - mutex, queue, region, counting sems,
 > whatever). I was just wondering if we could make the *tex thingy a bit more
 > stateful to ease the job for the skins, just in case we would use it for mutexes
 > only. I have no immediate answer to this question, just asking -- this is my
 > contribution as a senior member of the peanut gallery.

We can certainly implement an abstraction managing xnarch_atomic_t +
xnsynch_t, however, it seems that we would have to re-factor all
mutex/semaphores implementations to use this new abstraction. The
current approach is to add an xnarch_atomic_cmpxchg in user-space, and
fall back to an almost unchanged kernel-space support when it fails.

 > 
 > >  > 
 > >  >  I feel this
 > >  > > would complicate things: currently, the way I implemented user-space
 > >  > > mutexes for the posix skin kept the old association between the
 > >  > > user-space mutex, and its kernel-space companion, also used by
 > >  > > kernel-space operations.
 > >  > > 
 > >  > 
 > >  > My concern boils down to: how much of the POSIX implementation, beyond the
 > >  > cb_lock stuff, would have to be duplicated to get the same support ported to,
 > >  > say the VxWorks semM services?
 > > 
 > > The initialization code, and the additional calls to
 > > xnarch_atomic_cmpxchg in user-space. If xnarch_atomic_cmpxchg fails we
 > > call kernel-space, which is mostly unchanged.
 > > 
 > > Because of the cb_lock stuff, I also needed to implement the
 > > kernel-space syscalls in two versions: one if user-space has
 > > xnarch_atomic_cmpxchg and could lock the mutex control block, the other
 > > if the mutex control block needs to be locked by kernel-space.
 > > 
 > 
 > This is the part my laziness would very much like to factor as much as possible.
 > If ever possible.

The current implementation is more or less:

/* Common generic function */
pse51_mutex_thing_inner(mutex)
{
    /*... Assume mutex cb is locked ... */
}

/* Kernel-space operation */
pthread_mutex_thing(mutex)
{
     cb_trylock(mutex->lock);
  
     pse51_mutex_thing_inner(mutex);

     cb_unlock(mutex->lock);
}

/* syscall wrapper */
#ifdef XNARCH_HAVE_US_ATOMIC_CMPXCHG
__pthread_mutex_thing()
{
     copy_from_user();

     pse51_mutex_thing_inner(mutex)

     copy_to_user();
}
#else
__pthread_mutex_thing()
{
     copy_from_user();

     pthread_mutex_thing(mutex)

     copy_to_user();
}
#endif


-- 


					    Gilles.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Xenomai-core] [Patch 5/7] Define new syscalls for the system skin
  2008-05-18 22:52                     ` Gilles Chanteperdrix
@ 2008-05-19 12:56                       ` Philippe Gerum
  2008-05-19 13:16                         ` Gilles Chanteperdrix
  0 siblings, 1 reply; 34+ messages in thread
From: Philippe Gerum @ 2008-05-19 12:56 UTC (permalink / raw)
  To: Gilles Chanteperdrix; +Cc: xenomai

Gilles Chanteperdrix wrote:
> Philippe Gerum wrote:
>  > Gilles Chanteperdrix wrote:
>  > > As far as I understood, the user-space atomic operations, used to
>  > > acquire a free mutex in user-space, are not part of the futex API. In
>  > > our case, we are using xnarch_atomic_* operations to implement portably
>  > > this user-space locking stuff. I think that even setting the bit saying
>  > > that the mutex is currently owned is done in pthread_mutexes
>  > > implementation, not in the Futex API.
>  > 
>  > I would fully agree if the futex API did not define PI-based ops, which are
>  > needed for proper real-time operations from userland. You will certainly agree
>  > that PI implies that some kind of ownership exists; and because there can't be
>  > more than a single owner in that case, you end up with an object that can't be
>  > held by more than a single task. So you do have a mutex in disguise, whatever
>  > the way to keep its state is (a bit, an integer, whatever). So there is stronger
>  > semantics attached to that API than to simply manage an event notification scheme.
>  > 
>  >  Now, what remains is
>  > > sys_futex(FUTEX_WAIT) and sys_futex(FUTEX_WAKE), this terribly looks like
>  > > xnsync_sleep_on and xnsynch_wakeup_one_sleeper.
>  > > 
>  > 
>  > Yes, here again I partially agree, except for a significant issue: xnsynch is a
>  > stateless object (that's why we can use it for different syncobjs which are
>  > totally unrelated in their semantics - mutex, queue, region, counting sems,
>  > whatever). I was just wondering if we could make the *tex thingy a bit more
>  > stateful to ease the job for the skins, just in case we would use it for mutexes
>  > only. I have no immediate answer to this question, just asking -- this is my
>  > contribution as a senior member of the peanut gallery.
> 
> We can certainly implement an abstraction managing xnarch_atomic_t +
> xnsynch_t, however, it seems that we would have to re-factor all
> mutex/semaphores implementations to use this new abstraction. The
> current approach is to add an xnarch_atomic_cmpxchg in user-space, and
> fall back to an almost unchanged kernel-space support when it fails.
>

Ok. Let's merge this as it is. Common code will emerge eventually if it happens
to make sense when plugging the feature into the native and VxWorks mutex support.

>  > 
>  > >  > 
>  > >  >  I feel this
>  > >  > > would complicate things: currently, the way I implemented user-space
>  > >  > > mutexes for the posix skin kept the old association between the
>  > >  > > user-space mutex, and its kernel-space companion, also used by
>  > >  > > kernel-space operations.
>  > >  > > 
>  > >  > 
>  > >  > My concern boils down to: how much of the POSIX implementation, beyond the
>  > >  > cb_lock stuff, would have to be duplicated to get the same support ported to,
>  > >  > say the VxWorks semM services?
>  > > 
>  > > The initialization code, and the additional calls to
>  > > xnarch_atomic_cmpxchg in user-space. If xnarch_atomic_cmpxchg fails we
>  > > call kernel-space, which is mostly unchanged.
>  > > 
>  > > Because of the cb_lock stuff, I also needed to implement the
>  > > kernel-space syscalls in two versions: one if user-space has
>  > > xnarch_atomic_cmpxchg and could lock the mutex control block, the other
>  > > if the mutex control block needs to be locked by kernel-space.
>  > > 
>  > 
>  > This is the part my laziness would very much like to factor as much as possible.
>  > If ever possible.
> 
> The current implementation is more or less:
> 
> /* Common generic function */
> pse51_mutex_thing_inner(mutex)
> {
>     /*... Assume mutex cb is locked ... */
> }
> 
> /* Kernel-space operation */
> pthread_mutex_thing(mutex)
> {
>      cb_trylock(mutex->lock);
>   
>      pse51_mutex_thing_inner(mutex);
> 
>      cb_unlock(mutex->lock);
> }
> 
> /* syscall wrapper */
> #ifdef XNARCH_HAVE_US_ATOMIC_CMPXCHG
> __pthread_mutex_thing()
> {
>      copy_from_user();
> 
>      pse51_mutex_thing_inner(mutex)
> 
>      copy_to_user();
> }
> #else
> __pthread_mutex_thing()
> {
>      copy_from_user();
> 
>      pthread_mutex_thing(mutex)
> 
>      copy_to_user();
> }
> #endif
> 
> 


-- 
Philippe.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Xenomai-core] [Patch 5/7] Define new syscalls for the system skin
  2008-05-19 12:56                       ` Philippe Gerum
@ 2008-05-19 13:16                         ` Gilles Chanteperdrix
  2008-05-19 22:44                           ` Gilles Chanteperdrix
  0 siblings, 1 reply; 34+ messages in thread
From: Gilles Chanteperdrix @ 2008-05-19 13:16 UTC (permalink / raw)
  To: rpm; +Cc: xenomai

On Mon, May 19, 2008 at 2:56 PM, Philippe Gerum <rpm@xenomai.org> wrote:
> Gilles Chanteperdrix wrote:
>> Philippe Gerum wrote:
>>  > Gilles Chanteperdrix wrote:
>>  > > As far as I understood, the user-space atomic operations, used to
>>  > > acquire a free mutex in user-space, are not part of the futex API. In
>>  > > our case, we are using xnarch_atomic_* operations to implement portably
>>  > > this user-space locking stuff. I think that even setting the bit saying
>>  > > that the mutex is currently owned is done in pthread_mutexes
>>  > > implementation, not in the Futex API.
>>  >
>>  > I would fully agree if the futex API did not define PI-based ops, which are
>>  > needed for proper real-time operations from userland. You will certainly agree
>>  > that PI implies that some kind of ownership exists; and because there can't be
>>  > more than a single owner in that case, you end up with an object that can't be
>>  > held by more than a single task. So you do have a mutex in disguise, whatever
>>  > the way to keep its state is (a bit, an integer, whatever). So there is stronger
>>  > semantics attached to that API than to simply manage an event notification scheme.
>>  >
>>  >  Now, what remains is
>>  > > sys_futex(FUTEX_WAIT) and sys_futex(FUTEX_WAKE), this terribly looks like
>>  > > xnsync_sleep_on and xnsynch_wakeup_one_sleeper.
>>  > >
>>  >
>>  > Yes, here again I partially agree, except for a significant issue: xnsynch is a
>>  > stateless object (that's why we can use it for different syncobjs which are
>>  > totally unrelated in their semantics - mutex, queue, region, counting sems,
>>  > whatever). I was just wondering if we could make the *tex thingy a bit more
>>  > stateful to ease the job for the skins, just in case we would use it for mutexes
>>  > only. I have no immediate answer to this question, just asking -- this is my
>>  > contribution as a senior member of the peanut gallery.
>>
>> We can certainly implement an abstraction managing xnarch_atomic_t +
>> xnsynch_t, however, it seems that we would have to re-factor all
>> mutex/semaphores implementations to use this new abstraction. The
>> current approach is to add an xnarch_atomic_cmpxchg in user-space, and
>> fall back to an almost unchanged kernel-space support when it fails.
>>
>
> Ok. Let's merge this as it is. Common code will emerge eventually if it happens
> to make sense when plugging the feature into the native and VxWorks mutex support.

Ok. I need a few more days though, to adapt it to other architectures than arm.

-- 
 Gilles


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Xenomai-core] [Patch 5/7] Define new syscalls for the system skin
  2008-05-19 13:16                         ` Gilles Chanteperdrix
@ 2008-05-19 22:44                           ` Gilles Chanteperdrix
  0 siblings, 0 replies; 34+ messages in thread
From: Gilles Chanteperdrix @ 2008-05-19 22:44 UTC (permalink / raw)
  To: rpm, xenomai

Gilles Chanteperdrix wrote:
 > On Mon, May 19, 2008 at 2:56 PM, Philippe Gerum <rpm@xenomai.org> wrote:
 > > Gilles Chanteperdrix wrote:
 > >> Philippe Gerum wrote:
 > >>  > Gilles Chanteperdrix wrote:
 > >>  > > As far as I understood, the user-space atomic operations, used to
 > >>  > > acquire a free mutex in user-space, are not part of the futex API. In
 > >>  > > our case, we are using xnarch_atomic_* operations to implement portably
 > >>  > > this user-space locking stuff. I think that even setting the bit saying
 > >>  > > that the mutex is currently owned is done in pthread_mutexes
 > >>  > > implementation, not in the Futex API.
 > >>  >
 > >>  > I would fully agree if the futex API did not define PI-based ops, which are
 > >>  > needed for proper real-time operations from userland. You will certainly agree
 > >>  > that PI implies that some kind of ownership exists; and because there can't be
 > >>  > more than a single owner in that case, you end up with an object that can't be
 > >>  > held by more than a single task. So you do have a mutex in disguise, whatever
 > >>  > the way to keep its state is (a bit, an integer, whatever). So there is stronger
 > >>  > semantics attached to that API than to simply manage an event notification scheme.
 > >>  >
 > >>  >  Now, what remains is
 > >>  > > sys_futex(FUTEX_WAIT) and sys_futex(FUTEX_WAKE), this terribly looks like
 > >>  > > xnsync_sleep_on and xnsynch_wakeup_one_sleeper.
 > >>  > >
 > >>  >
 > >>  > Yes, here again I partially agree, except for a significant issue: xnsynch is a
 > >>  > stateless object (that's why we can use it for different syncobjs which are
 > >>  > totally unrelated in their semantics - mutex, queue, region, counting sems,
 > >>  > whatever). I was just wondering if we could make the *tex thingy a bit more
 > >>  > stateful to ease the job for the skins, just in case we would use it for mutexes
 > >>  > only. I have no immediate answer to this question, just asking -- this is my
 > >>  > contribution as a senior member of the peanut gallery.
 > >>
 > >> We can certainly implement an abstraction managing xnarch_atomic_t +
 > >> xnsynch_t, however, it seems that we would have to re-factor all
 > >> mutex/semaphores implementations to use this new abstraction. The
 > >> current approach is to add an xnarch_atomic_cmpxchg in user-space, and
 > >> fall back to an almost unchanged kernel-space support when it fails.
 > >>
 > >
 > > Ok. Let's merge this as it is. Common code will emerge eventually if it happens
 > > to make sense when plugging the feature into the native and VxWorks mutex support.
 > 
 > Ok. I need a few more days though, to adapt it to other architectures than arm.

I changed my mind and commited the whole stuff. Instead of implementing
full atomic operations in user-space for other platforms than ARM, I
have left this work for others.

-- 


					    Gilles.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Xenomai-core] [Patch 7/7] Re-implementation of mutexes, user-space support.
  2008-05-18 18:56                           ` Philippe Gerum
@ 2008-05-19 22:49                             ` Gilles Chanteperdrix
  2008-05-20  6:53                               ` Philippe Gerum
  0 siblings, 1 reply; 34+ messages in thread
From: Gilles Chanteperdrix @ 2008-05-19 22:49 UTC (permalink / raw)
  To: rpm; +Cc: Jan Kiszka, xenomai

Philippe Gerum wrote:
 > Gilles Chanteperdrix wrote:
 > > Jan Kiszka wrote:
 > >  > Gilles Chanteperdrix wrote:
 > >  > > Jan Kiszka wrote:
 > >  > >  > Gilles Chanteperdrix wrote:
 > >  > >  > > Philippe Gerum wrote:
 > >  > >  > >  > Gilles Chanteperdrix wrote:
 > >  > >  > >  > > Since binding of the semaphore heaps is now made by xeno_skin_bind, there is
 > >  > >  > >  > > much less modifications in src/skins/posix/init.c. However, I had to do
 > >  > >  > >  > > something really ugly: since binding the semaphore heaps by xeno_skin_bind
 > >  > >  > >  > > requires calls to open, ioctl, mmap, close and munmap, I redefined these symbols
 > >  > >  > >  > > to be the __real_ variants before including bind.h.
 > >  > >  > >  > 
 > >  > >  > >  > Is there any upside to do this instead of simply calling the __real_*
 > >  > >  > >  > placeholders, since we do already provide weak wrappers for those when the
 > >  > >  > >  > linker's wrapping magic is not invoked?
 > >  > >  > > 
 > >  > >  > > The point is that the wrappers and linker magic only take place for
 > >  > >  > > POSIX skins. Other skins have to call the genuine open, ioctl, mmap,
 > >  > >  > > close and munmap services.
 > >  > >  > 
 > >  > >  > What about controlling the desired prefix for generic functions in
 > >  > >  > bind.h via some #define that the caller has to/can set before including
 > >  > >  > the header:
 > >  > >  > 
 > >  > >  > #define POSIX_PREFIX	__real_
 > >  > >  > #include <asm-generic/bits/bind.h>
 > >  > >  > 
 > >  > >  > and there we would have:
 > >  > >  > 
 > >  > >  > POSIX_PREFIX##open(...);
 > >  > >  > 
 > >  > >  > Looks a bit cleaner to me.
 > >  > > 
 > >  > > Well, in this case we end up cluttering the code with the POSIX_PREFIX
 > >  > > macro, even in the non posix case where no prefix is needed.
 > >  > 
 > >  > Yes, but there are only few spots. The advantage of this strategy is
 > >  > that it is explicit in-place (ie. inside bind.h). That avoids potential
 > >  > collateral damage in the future when other services are added to that
 > >  > helper which shall not be wrapped.
 > > 
 > > Actually, the only important call is open. Since once the file
 > > descriptor has been created with __real_open, all syscall wrappers will
 > > automatically fall back to the __real syscall variants.
 > > 
 > 
 > Then let's just provide __real_open() as a weak symbol in all libs; that's less
 > error-prone that fiddling with the preprocessor.

For now, I kept a solution based on the preprocessor. If I remember
correctly, the __real stuff needs to be in a separate object, this
means that we cannot put __real_open definition in bind.h, we have to
generate an open_wrapper.lo object and link every library with this
object. Is this really what we want ?

-- 


					    Gilles.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Xenomai-core] [Patch 7/7] Re-implementation of mutexes, user-space support.
  2008-05-19 22:49                             ` Gilles Chanteperdrix
@ 2008-05-20  6:53                               ` Philippe Gerum
  2008-05-20  7:07                                 ` Jan Kiszka
  0 siblings, 1 reply; 34+ messages in thread
From: Philippe Gerum @ 2008-05-20  6:53 UTC (permalink / raw)
  To: Gilles Chanteperdrix; +Cc: Jan Kiszka, xenomai

Gilles Chanteperdrix wrote:
> Philippe Gerum wrote:
>  > Gilles Chanteperdrix wrote:
>  > > Jan Kiszka wrote:
>  > >  > Gilles Chanteperdrix wrote:
>  > >  > > Jan Kiszka wrote:
>  > >  > >  > Gilles Chanteperdrix wrote:
>  > >  > >  > > Philippe Gerum wrote:
>  > >  > >  > >  > Gilles Chanteperdrix wrote:
>  > >  > >  > >  > > Since binding of the semaphore heaps is now made by xeno_skin_bind, there is
>  > >  > >  > >  > > much less modifications in src/skins/posix/init.c. However, I had to do
>  > >  > >  > >  > > something really ugly: since binding the semaphore heaps by xeno_skin_bind
>  > >  > >  > >  > > requires calls to open, ioctl, mmap, close and munmap, I redefined these symbols
>  > >  > >  > >  > > to be the __real_ variants before including bind.h.
>  > >  > >  > >  > 
>  > >  > >  > >  > Is there any upside to do this instead of simply calling the __real_*
>  > >  > >  > >  > placeholders, since we do already provide weak wrappers for those when the
>  > >  > >  > >  > linker's wrapping magic is not invoked?
>  > >  > >  > > 
>  > >  > >  > > The point is that the wrappers and linker magic only take place for
>  > >  > >  > > POSIX skins. Other skins have to call the genuine open, ioctl, mmap,
>  > >  > >  > > close and munmap services.
>  > >  > >  > 
>  > >  > >  > What about controlling the desired prefix for generic functions in
>  > >  > >  > bind.h via some #define that the caller has to/can set before including
>  > >  > >  > the header:
>  > >  > >  > 
>  > >  > >  > #define POSIX_PREFIX	__real_
>  > >  > >  > #include <asm-generic/bits/bind.h>
>  > >  > >  > 
>  > >  > >  > and there we would have:
>  > >  > >  > 
>  > >  > >  > POSIX_PREFIX##open(...);
>  > >  > >  > 
>  > >  > >  > Looks a bit cleaner to me.
>  > >  > > 
>  > >  > > Well, in this case we end up cluttering the code with the POSIX_PREFIX
>  > >  > > macro, even in the non posix case where no prefix is needed.
>  > >  > 
>  > >  > Yes, but there are only few spots. The advantage of this strategy is
>  > >  > that it is explicit in-place (ie. inside bind.h). That avoids potential
>  > >  > collateral damage in the future when other services are added to that
>  > >  > helper which shall not be wrapped.
>  > > 
>  > > Actually, the only important call is open. Since once the file
>  > > descriptor has been created with __real_open, all syscall wrappers will
>  > > automatically fall back to the __real syscall variants.
>  > > 
>  > 
>  > Then let's just provide __real_open() as a weak symbol in all libs; that's less
>  > error-prone that fiddling with the preprocessor.
> 
> For now, I kept a solution based on the preprocessor. If I remember
> correctly, the __real stuff needs to be in a separate object, this
> means that we cannot put __real_open definition in bind.h, we have to
> generate an open_wrapper.lo object and link every library with this
> object. Is this really what we want ?
> 

bind.h is meant to be included exclusively from init.c files. So yes, as far as
possible, we don't want to mess with the preprocessor.

-- 
Philippe.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Xenomai-core] [Patch 7/7] Re-implementation of mutexes, user-space support.
  2008-05-20  6:53                               ` Philippe Gerum
@ 2008-05-20  7:07                                 ` Jan Kiszka
  2008-05-20  7:23                                   ` Gilles Chanteperdrix
  0 siblings, 1 reply; 34+ messages in thread
From: Jan Kiszka @ 2008-05-20  7:07 UTC (permalink / raw)
  To: rpm; +Cc: xenomai

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

Philippe Gerum wrote:
> Gilles Chanteperdrix wrote:
>> Philippe Gerum wrote:
>>  > Gilles Chanteperdrix wrote:
>>  > > Jan Kiszka wrote:
>>  > >  > Gilles Chanteperdrix wrote:
>>  > >  > > Jan Kiszka wrote:
>>  > >  > >  > Gilles Chanteperdrix wrote:
>>  > >  > >  > > Philippe Gerum wrote:
>>  > >  > >  > >  > Gilles Chanteperdrix wrote:
>>  > >  > >  > >  > > Since binding of the semaphore heaps is now made by xeno_skin_bind, there is
>>  > >  > >  > >  > > much less modifications in src/skins/posix/init.c. However, I had to do
>>  > >  > >  > >  > > something really ugly: since binding the semaphore heaps by xeno_skin_bind
>>  > >  > >  > >  > > requires calls to open, ioctl, mmap, close and munmap, I redefined these symbols
>>  > >  > >  > >  > > to be the __real_ variants before including bind.h.
>>  > >  > >  > >  > 
>>  > >  > >  > >  > Is there any upside to do this instead of simply calling the __real_*
>>  > >  > >  > >  > placeholders, since we do already provide weak wrappers for those when the
>>  > >  > >  > >  > linker's wrapping magic is not invoked?
>>  > >  > >  > > 
>>  > >  > >  > > The point is that the wrappers and linker magic only take place for
>>  > >  > >  > > POSIX skins. Other skins have to call the genuine open, ioctl, mmap,
>>  > >  > >  > > close and munmap services.
>>  > >  > >  > 
>>  > >  > >  > What about controlling the desired prefix for generic functions in
>>  > >  > >  > bind.h via some #define that the caller has to/can set before including
>>  > >  > >  > the header:
>>  > >  > >  > 
>>  > >  > >  > #define POSIX_PREFIX	__real_
>>  > >  > >  > #include <asm-generic/bits/bind.h>
>>  > >  > >  > 
>>  > >  > >  > and there we would have:
>>  > >  > >  > 
>>  > >  > >  > POSIX_PREFIX##open(...);
>>  > >  > >  > 
>>  > >  > >  > Looks a bit cleaner to me.
>>  > >  > > 
>>  > >  > > Well, in this case we end up cluttering the code with the POSIX_PREFIX
>>  > >  > > macro, even in the non posix case where no prefix is needed.
>>  > >  > 
>>  > >  > Yes, but there are only few spots. The advantage of this strategy is
>>  > >  > that it is explicit in-place (ie. inside bind.h). That avoids potential
>>  > >  > collateral damage in the future when other services are added to that
>>  > >  > helper which shall not be wrapped.
>>  > > 
>>  > > Actually, the only important call is open. Since once the file
>>  > > descriptor has been created with __real_open, all syscall wrappers will
>>  > > automatically fall back to the __real syscall variants.
>>  > > 
>>  > 
>>  > Then let's just provide __real_open() as a weak symbol in all libs; that's less
>>  > error-prone that fiddling with the preprocessor.
>>
>> For now, I kept a solution based on the preprocessor. If I remember
>> correctly, the __real stuff needs to be in a separate object, this
>> means that we cannot put __real_open definition in bind.h, we have to
>> generate an open_wrapper.lo object and link every library with this
>> object. Is this really what we want ?
>>
> 
> bind.h is meant to be included exclusively from init.c files. So yes, as far as
> possible, we don't want to mess with the preprocessor.

Then better let the caller decide which version to call. A simple
argument to xeno_bind_skin can be passed through to map_sem_heap and
define there if open or __real_open is used. As the functions are
inlined, we shouldn't see the other symbol after the compilation.

Jan


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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Xenomai-core] [Patch 7/7] Re-implementation of mutexes, user-space support.
  2008-05-20  7:07                                 ` Jan Kiszka
@ 2008-05-20  7:23                                   ` Gilles Chanteperdrix
  0 siblings, 0 replies; 34+ messages in thread
From: Gilles Chanteperdrix @ 2008-05-20  7:23 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: xenomai

On Tue, May 20, 2008 at 9:07 AM, Jan Kiszka <jan.kiszka@domain.hid> wrote:
> Philippe Gerum wrote:
>> Gilles Chanteperdrix wrote:
>>> Philippe Gerum wrote:
>>>  > Gilles Chanteperdrix wrote:
>>>  > > Jan Kiszka wrote:
>>>  > >  > Gilles Chanteperdrix wrote:
>>>  > >  > > Jan Kiszka wrote:
>>>  > >  > >  > Gilles Chanteperdrix wrote:
>>>  > >  > >  > > Philippe Gerum wrote:
>>>  > >  > >  > >  > Gilles Chanteperdrix wrote:
>>>  > >  > >  > >  > > Since binding of the semaphore heaps is now made by xeno_skin_bind, there is
>>>  > >  > >  > >  > > much less modifications in src/skins/posix/init.c. However, I had to do
>>>  > >  > >  > >  > > something really ugly: since binding the semaphore heaps by xeno_skin_bind
>>>  > >  > >  > >  > > requires calls to open, ioctl, mmap, close and munmap, I redefined these symbols
>>>  > >  > >  > >  > > to be the __real_ variants before including bind.h.
>>>  > >  > >  > >  >
>>>  > >  > >  > >  > Is there any upside to do this instead of simply calling the __real_*
>>>  > >  > >  > >  > placeholders, since we do already provide weak wrappers for those when the
>>>  > >  > >  > >  > linker's wrapping magic is not invoked?
>>>  > >  > >  > >
>>>  > >  > >  > > The point is that the wrappers and linker magic only take place for
>>>  > >  > >  > > POSIX skins. Other skins have to call the genuine open, ioctl, mmap,
>>>  > >  > >  > > close and munmap services.
>>>  > >  > >  >
>>>  > >  > >  > What about controlling the desired prefix for generic functions in
>>>  > >  > >  > bind.h via some #define that the caller has to/can set before including
>>>  > >  > >  > the header:
>>>  > >  > >  >
>>>  > >  > >  > #define POSIX_PREFIX    __real_
>>>  > >  > >  > #include <asm-generic/bits/bind.h>
>>>  > >  > >  >
>>>  > >  > >  > and there we would have:
>>>  > >  > >  >
>>>  > >  > >  > POSIX_PREFIX##open(...);
>>>  > >  > >  >
>>>  > >  > >  > Looks a bit cleaner to me.
>>>  > >  > >
>>>  > >  > > Well, in this case we end up cluttering the code with the POSIX_PREFIX
>>>  > >  > > macro, even in the non posix case where no prefix is needed.
>>>  > >  >
>>>  > >  > Yes, but there are only few spots. The advantage of this strategy is
>>>  > >  > that it is explicit in-place (ie. inside bind.h). That avoids potential
>>>  > >  > collateral damage in the future when other services are added to that
>>>  > >  > helper which shall not be wrapped.
>>>  > >
>>>  > > Actually, the only important call is open. Since once the file
>>>  > > descriptor has been created with __real_open, all syscall wrappers will
>>>  > > automatically fall back to the __real syscall variants.
>>>  > >
>>>  >
>>>  > Then let's just provide __real_open() as a weak symbol in all libs; that's less
>>>  > error-prone that fiddling with the preprocessor.
>>>
>>> For now, I kept a solution based on the preprocessor. If I remember
>>> correctly, the __real stuff needs to be in a separate object, this
>>> means that we cannot put __real_open definition in bind.h, we have to
>>> generate an open_wrapper.lo object and link every library with this
>>> object. Is this really what we want ?
>>>
>>
>> bind.h is meant to be included exclusively from init.c files. So yes, as far as
>> possible, we don't want to mess with the preprocessor.
>
> Then better let the caller decide which version to call. A simple
> argument to xeno_bind_skin can be passed through to map_sem_heap and
> define there if open or __real_open is used. As the functions are
> inlined, we shouldn't see the other symbol after the compilation.

Except the day when would compile without optimization... I will
implement the open_wrapper.lo solution.

-- 
 Gilles


^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2008-05-20  7:23 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-05-02 22:27 [Xenomai-core] [Patch 0/7] Posix skin user-space mutexes, second take Gilles Chanteperdrix
2008-05-02 22:30 ` [Xenomai-core] [Patch 1/7] Support for non cached memory mappings Gilles Chanteperdrix
2008-05-02 22:32   ` [Xenomai-core] [Patch 2/7] Define XNARCH_SHARED_HEAP_FLAGS Gilles Chanteperdrix
2008-05-02 22:33     ` [Xenomai-core] [Patch 3/7] Define more atomic operations in user-space Gilles Chanteperdrix
2008-05-02 22:34       ` [Xenomai-core] [Patch 4/7] Define ARM " Gilles Chanteperdrix
2008-05-02 22:35         ` [Xenomai-core] [Patch 5/7] Define new syscalls for the system skin Gilles Chanteperdrix
2008-05-02 22:36           ` [Xenomai-core] [Patch 6/7] Re-implementation of mutexes, kernel-space support Gilles Chanteperdrix
2008-05-02 22:38             ` [Xenomai-core] [Patch 7/7] Re-implementation of mutexes, user-space support Gilles Chanteperdrix
2008-05-18 16:42               ` Philippe Gerum
2008-05-18 17:05                 ` Gilles Chanteperdrix
2008-05-18 17:29                   ` Jan Kiszka
2008-05-18 17:41                     ` Gilles Chanteperdrix
2008-05-18 17:52                       ` Jan Kiszka
2008-05-18 18:09                         ` Gilles Chanteperdrix
2008-05-18 18:56                           ` Philippe Gerum
2008-05-19 22:49                             ` Gilles Chanteperdrix
2008-05-20  6:53                               ` Philippe Gerum
2008-05-20  7:07                                 ` Jan Kiszka
2008-05-20  7:23                                   ` Gilles Chanteperdrix
2008-05-18 16:40           ` [Xenomai-core] [Patch 5/7] Define new syscalls for the system skin Philippe Gerum
2008-05-18 17:21             ` Gilles Chanteperdrix
2008-05-18 18:37             ` Gilles Chanteperdrix
2008-05-18 19:10               ` Philippe Gerum
2008-05-18 19:38                 ` Gilles Chanteperdrix
2008-05-18 21:52                   ` Philippe Gerum
2008-05-18 22:52                     ` Gilles Chanteperdrix
2008-05-19 12:56                       ` Philippe Gerum
2008-05-19 13:16                         ` Gilles Chanteperdrix
2008-05-19 22:44                           ` Gilles Chanteperdrix
2008-05-05 16:33         ` [Xenomai-core] [Patch 4/7] Define ARM atomic operations in user-space Gilles Chanteperdrix
2008-05-05 16:39           ` Philippe Gerum
2008-05-05 16:44             ` Gilles Chanteperdrix
2008-05-18 16:30   ` [Xenomai-core] [Patch 1/7] Support for non cached memory mappings Philippe Gerum
2008-05-03 19:18 ` [Xenomai-core] [Patch 0/7] Posix skin user-space mutexes, second take Gilles Chanteperdrix

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.