[Xenomai-core] [PATCH v2 0/3] Sem heap statistics & mutex auto-cleanup fixes

[Xenomai-core] [PATCH v2 0/3] Sem heap statistics & mutex auto-cleanup fixes

[Jan Kiszka]

This version extends the heap statistics to include all heaps that are
intialized by the nucleus or some skip. Some internal API modifications
were required which makes the second patch a bit larger. Moreover, I
modified some internal locking of nucleus' heap to use plain Linux spin
locks instead of heavy nklock.

Please pull the series from

    git://xenomai.org/xenomai-jki.git for-upstream

if there are no concerns.

Jan Kiszka (3):
      nucleus: Use Linux spin lock for heap list management
      nucleus: Include all heaps in statistics
      native: Release fastlock to the proper heap

 include/nucleus/heap.h    |   12 ++++-
 ksrc/drivers/ipc/iddp.c   |    3 +-
 ksrc/drivers/ipc/xddp.c   |    6 ++-
 ksrc/nucleus/heap.c       |  132 ++++++++++++++++++++++++++++++++++----------
 ksrc/nucleus/module.c     |    2 +-
 ksrc/nucleus/pod.c        |    5 +-
 ksrc/nucleus/shadow.c     |    5 ++-
 ksrc/skins/native/heap.c  |    6 ++-
 ksrc/skins/native/mutex.c |   14 ++++-
 ksrc/skins/native/pipe.c  |    4 +-
 ksrc/skins/native/queue.c |    6 ++-
 ksrc/skins/posix/shm.c    |    4 +-
 ksrc/skins/psos+/rn.c     |    6 ++-
 ksrc/skins/rtai/shm.c     |    7 ++-
 ksrc/skins/vrtx/heap.c    |    6 ++-
 ksrc/skins/vrtx/syscall.c |    3 +-
 16 files changed, 167 insertions(+), 54 deletions(-)

[1] http://thread.gmane.org/gmane.linux.real-time.xenomai.devel/6559

[Xenomai-core] [PATCH v2 1/3] nucleus: Use Linux spin lock for heap list management

[Jan Kiszka]

No need for hard nklock protection of kheapq and the map counter, a
normal spin lock suffices as all users must run over the root thread
anyway.

Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid>
---

 ksrc/nucleus/heap.c |   26 ++++++++++++--------------
 1 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/ksrc/nucleus/heap.c b/ksrc/nucleus/heap.c
index 9ca2591..5a17a94 100644
--- a/ksrc/nucleus/heap.c
+++ b/ksrc/nucleus/heap.c
@@ -76,6 +76,8 @@ EXPORT_SYMBOL_GPL(kheap);
 xnheap_t kstacks;	/* Private stack pool */
 #endif
 
+static DEFINE_SPINLOCK(heapq_lock);
+
 static void init_extent(xnheap_t *heap, xnextent_t *extent)
 {
 	caddr_t freepage;
@@ -1022,14 +1024,13 @@ static void __unreserve_and_free_heap(void *ptr, size_t size, int kmflags)
 static void xnheap_vmclose(struct vm_area_struct *vma)
 {
 	xnheap_t *heap = vma->vm_private_data;
-	spl_t s;
 
-	xnlock_get_irqsave(&nklock, s);
+	spin_lock(&heapq_lock);
 
 	if (atomic_dec_and_test(&heap->archdep.numaps)) {
 		if (heap->archdep.release) {
 			removeq(&kheapq, &heap->link);
-			xnlock_put_irqrestore(&nklock, s);
+			spin_unlock(&heapq_lock);
 			__unreserve_and_free_heap(heap->archdep.heapbase,
 						  xnheap_extentsize(heap),
 						  heap->archdep.kmflags);
@@ -1038,7 +1039,7 @@ static void xnheap_vmclose(struct vm_area_struct *vma)
 		}
 	}
 
-	xnlock_put_irqrestore(&nklock, s);
+	spin_unlock(&heapq_lock);
 }
 
 static struct vm_operations_struct xnheap_vmops = {
@@ -1068,9 +1069,8 @@ static int xnheap_ioctl(struct inode *inode,
 {
 	xnheap_t *heap;
 	int err = 0;
-	spl_t s;
 
-	xnlock_get_irqsave(&nklock, s);
+	spin_lock(&heapq_lock);
 
 	heap = __validate_heap_addr((void *)arg);
 
@@ -1083,7 +1083,7 @@ static int xnheap_ioctl(struct inode *inode,
 
       unlock_and_exit:
 
-	xnlock_put_irqrestore(&nklock, s);
+	spin_unlock(&heapq_lock);
 
 	return err;
 }
@@ -1148,7 +1148,6 @@ static int xnheap_mmap(struct file *file, struct vm_area_struct *vma)
 int xnheap_init_mapped(xnheap_t *heap, u_long heapsize, int memflags)
 {
 	void *heapbase;
-	spl_t s;
 	int err;
 
 	/* Caller must have accounted for internal overhead. */
@@ -1172,9 +1171,9 @@ int xnheap_init_mapped(xnheap_t *heap, u_long heapsize, int memflags)
 	heap->archdep.heapbase = heapbase;
 	heap->archdep.release = NULL;
 
-	xnlock_get_irqsave(&nklock, s);
+	spin_lock(&heapq_lock);
 	appendq(&kheapq, &heap->link);
-	xnlock_put_irqrestore(&nklock, s);
+	spin_unlock(&heapq_lock);
 
 	return 0;
 }
@@ -1184,20 +1183,19 @@ int xnheap_destroy_mapped(xnheap_t *heap, void (*release)(struct xnheap *heap),
 {
 	int ret = 0, ccheck;
 	unsigned long len;
-	spl_t s;
 
 	ccheck = mapaddr ? 1 : 0;
 
-	xnlock_get_irqsave(&nklock, s);
+	spin_lock(&heapq_lock);
 
 	if (atomic_read(&heap->archdep.numaps) > ccheck) {
 		heap->archdep.release = release;
-		xnlock_put_irqrestore(&nklock, s);
+		spin_unlock(&heapq_lock);
 		return -EBUSY;
 	}
 
 	removeq(&kheapq, &heap->link); /* Prevent further mapping. */
-	xnlock_put_irqrestore(&nklock, s);
+	spin_unlock(&heapq_lock);
 
 	len = xnheap_extentsize(heap);
 

Re: [Xenomai-core] [PATCH v2 1/3] nucleus: Use Linux spin lock for heap list management

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> No need for hard nklock protection of kheapq and the map counter, a
> normal spin lock suffices as all users must run over the root thread
> anyway.

At the very least, this should use rthal_spin_lock, in order to seem to
respect the layering.

Anyway, do we really want to change this now?

-- 
					    Gilles.

Re: [Xenomai-core] [PATCH v2 1/3] nucleus: Use Linux spin lock for heap list management

[Jan Kiszka]

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

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> No need for hard nklock protection of kheapq and the map counter, a
>> normal spin lock suffices as all users must run over the root thread
>> anyway.
> 
> At the very least, this should use rthal_spin_lock, in order to seem to
> respect the layering.

Then the conversion would make no sense (hard interrupt lock again).
Given that we are fiddling with Linux mm directly here, there is no hal
involved.

> 
> Anyway, do we really want to change this now?
> 

That's a different question. The patch alone does not buy us that much
when we cannot reuse the lock for the heapq.

Jan

PS: More invasive changes will come anyway to plug cleanup races in the
heap code.


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

Re: [Xenomai-core] [PATCH v2 1/3] nucleus: Use Linux spin lock for heap list management

[Jan Kiszka]

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

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> No need for hard nklock protection of kheapq and the map counter, a
>>> normal spin lock suffices as all users must run over the root thread
>>> anyway.
>> At the very least, this should use rthal_spin_lock, in order to seem to
>> respect the layering.
> 
> Then the conversion would make no sense (hard interrupt lock again).
> Given that we are fiddling with Linux mm directly here, there is no hal
> involved.
> 
>> Anyway, do we really want to change this now?
>>
> 
> That's a different question. The patch alone does not buy us that much
> when we cannot reuse the lock for the heapq.

Wait, there is one advantage: We no longer have to walk kheapq under
nklock in __validate_heap_addr. I think that makes this patch
worthwhile. Will adapt it to avoid conflicts with the simulator.

> 
> Jan
> 
> PS: More invasive changes will come anyway to plug cleanup races in the
> heap code.
> 

Jan


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

[Xenomai-core] [PATCH v2 3/3] native: Release fastlock to the proper heap

[Jan Kiszka]

Don't assume rt_task_delete is only called for in-kernel users, it may
be triggered via auto-cleanup also on user space objects. So check for
the creator and release the fastlock to the correct heap.

Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid>
---

 ksrc/skins/native/mutex.c |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/ksrc/skins/native/mutex.c b/ksrc/skins/native/mutex.c
index 20eb484..6cf7eb1 100644
--- a/ksrc/skins/native/mutex.c
+++ b/ksrc/skins/native/mutex.c
@@ -47,6 +47,7 @@
 #include <nucleus/pod.h>
 #include <nucleus/registry.h>
 #include <nucleus/heap.h>
+#include <nucleus/sys_ppd.h>
 #include <native/task.h>
 #include <native/mutex.h>
 
@@ -316,8 +317,17 @@ int rt_mutex_delete(RT_MUTEX *mutex)
 	err = rt_mutex_delete_inner(mutex);
 
 #ifdef CONFIG_XENO_FASTSYNCH
-	if (!err)
-		xnfree(mutex->synch_base.fastlock);
+	if (!err) {
+#ifdef CONFIG_XENO_OPT_PERVASIVE
+		if (mutex->cpid) {
+			int global = xnsynch_test_flags(&mutex->synch_base,
+							RT_MUTEX_EXPORTED);
+			xnheap_free(&xnsys_ppd_get(global)->sem_heap,
+				    mutex->synch_base.fastlock);
+		} else
+#endif /* CONFIG_XENO_OPT_PERVASIVE */
+			xnfree(mutex->synch_base.fastlock);
+	}
 #endif /* CONFIG_XENO_FASTSYNCH */
 
 	return err;

[Xenomai-core] [PATCH v2 2/3] nucleus: Include all heaps in statistics

[Jan Kiszka]

This extends /proc/xenomai/heap with statistics about all currently used
heaps.

Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid>
---

 include/nucleus/heap.h    |   12 ++++-
 ksrc/drivers/ipc/iddp.c   |    3 +
 ksrc/drivers/ipc/xddp.c   |    6 ++-
 ksrc/nucleus/heap.c       |  106 ++++++++++++++++++++++++++++++++++++++-------
 ksrc/nucleus/module.c     |    2 -
 ksrc/nucleus/pod.c        |    5 +-
 ksrc/nucleus/shadow.c     |    5 ++
 ksrc/skins/native/heap.c  |    6 ++-
 ksrc/skins/native/pipe.c  |    4 +-
 ksrc/skins/native/queue.c |    6 ++-
 ksrc/skins/posix/shm.c    |    4 +-
 ksrc/skins/psos+/rn.c     |    6 ++-
 ksrc/skins/rtai/shm.c     |    7 ++-
 ksrc/skins/vrtx/heap.c    |    6 ++-
 ksrc/skins/vrtx/syscall.c |    3 +
 15 files changed, 143 insertions(+), 38 deletions(-)

diff --git a/include/nucleus/heap.h b/include/nucleus/heap.h
index f39ef64..fcd9a8d 100644
--- a/include/nucleus/heap.h
+++ b/include/nucleus/heap.h
@@ -115,6 +115,10 @@ typedef struct xnheap {
 
 	XNARCH_DECL_DISPLAY_CONTEXT();
 
+	xnholder_t stat_link;	/* Link in heapq */
+
+	char name[48];
+
 } xnheap_t;
 
 extern xnheap_t kheap;
@@ -202,7 +206,8 @@ void xnheap_cleanup_proc(void);
 
 int xnheap_init_mapped(xnheap_t *heap,
 		       u_long heapsize,
-		       int memflags);
+		       int memflags,
+		       const char *name, ...);
 
 int xnheap_destroy_mapped(xnheap_t *heap,
 			  void (*release)(struct xnheap *heap),
@@ -224,7 +229,10 @@ int xnheap_destroy_mapped(xnheap_t *heap,
 int xnheap_init(xnheap_t *heap,
 		void *heapaddr,
 		u_long heapsize,
-		u_long pagesize);
+		u_long pagesize,
+		const char *name, ...);
+
+void xnheap_set_name(xnheap_t *heap, const char *name, ...);
 
 int xnheap_destroy(xnheap_t *heap,
 		   void (*flushfn)(xnheap_t *heap,
diff --git a/ksrc/drivers/ipc/iddp.c b/ksrc/drivers/ipc/iddp.c
index a407946..b6382f1 100644
--- a/ksrc/drivers/ipc/iddp.c
+++ b/ksrc/drivers/ipc/iddp.c
@@ -559,7 +559,8 @@ static int __iddp_bind_socket(struct rtipc_private *priv,
 		}
 
 		ret = xnheap_init(&sk->privpool,
-				  poolmem, poolsz, XNHEAP_PAGE_SIZE);
+				  poolmem, poolsz, XNHEAP_PAGE_SIZE,
+				  "ippd: %d", port);
 		if (ret) {
 			xnarch_free_host_mem(poolmem, poolsz);
 			goto fail;
diff --git a/ksrc/drivers/ipc/xddp.c b/ksrc/drivers/ipc/xddp.c
index f62147a..a5dafef 100644
--- a/ksrc/drivers/ipc/xddp.c
+++ b/ksrc/drivers/ipc/xddp.c
@@ -703,7 +703,7 @@ static int __xddp_bind_socket(struct rtipc_private *priv,
 		}
 
 		ret = xnheap_init(&sk->privpool,
-				  poolmem, poolsz, XNHEAP_PAGE_SIZE);
+				  poolmem, poolsz, XNHEAP_PAGE_SIZE, "");
 		if (ret) {
 			xnarch_free_host_mem(poolmem, poolsz);
 			goto fail;
@@ -746,6 +746,10 @@ static int __xddp_bind_socket(struct rtipc_private *priv,
 	sk->minor = ret;
 	sa->sipc_port = ret;
 	sk->name = *sa;
+
+	if (poolsz > 0)
+		xnheap_set_name(sk->bufpool, "xddp: %d", sa->sipc_port);
+
 	/* Set default destination if unset at binding time. */
 	if (sk->peer.sipc_port < 0)
 		sk->peer = *sa;
diff --git a/ksrc/nucleus/heap.c b/ksrc/nucleus/heap.c
index 5a17a94..81fdd7a 100644
--- a/ksrc/nucleus/heap.c
+++ b/ksrc/nucleus/heap.c
@@ -77,6 +77,7 @@ xnheap_t kstacks;	/* Private stack pool */
 #endif
 
 static DEFINE_SPINLOCK(heapq_lock);
+static DEFINE_XNQUEUE(heapq);	/* Heap list for /proc reporting */
 
 static void init_extent(xnheap_t *heap, xnextent_t *extent)
 {
@@ -110,7 +111,7 @@ static void init_extent(xnheap_t *heap, xnextent_t *extent)
  */
 
 /*!
- * \fn xnheap_init(xnheap_t *heap,void *heapaddr,u_long heapsize,u_long pagesize)
+ * \fn xnheap_init(xnheap_t *heap,void *heapaddr,u_long heapsize,u_long pagesize,const char *name,...)
  * \brief Initialize a memory heap.
  *
  * Initializes a memory heap suitable for time-bounded allocation
@@ -147,6 +148,10 @@ static void init_extent(xnheap_t *heap, xnextent_t *extent)
  * best one for your needs. In the current implementation, pagesize
  * must be a power of two in the range [ 8 .. 32768 ] inclusive.
  *
+ * @param name Name displayed in statistic outputs. This parameter can
+ * be a format string, in which case succeeding parameters will be used
+ * to resolve the final name.
+ *
  * @return 0 is returned upon success, or one of the following error
  * codes:
  *
@@ -163,8 +168,8 @@ static void init_extent(xnheap_t *heap, xnextent_t *extent)
  * Rescheduling: never.
  */
 
-int xnheap_init(xnheap_t *heap,
-		void *heapaddr, u_long heapsize, u_long pagesize)
+static int xnheap_init_va(xnheap_t *heap, void *heapaddr, u_long heapsize,
+			  u_long pagesize, const char *name, va_list args)
 {
 	unsigned cpu, nr_cpus = xnarch_num_online_cpus();
 	u_long hdrsize, shiftsize, pageshift;
@@ -234,12 +239,65 @@ int xnheap_init(xnheap_t *heap,
 
 	appendq(&heap->extents, &extent->link);
 
+	vsnprintf(heap->name, sizeof(heap->name), name, args);
+
+	spin_lock(&heapq_lock);
+	appendq(&heapq, &heap->stat_link);
+	spin_unlock(&heapq_lock);
+
 	xnarch_init_display_context(heap);
 
 	return 0;
 }
+
+int xnheap_init(xnheap_t *heap, void *heapaddr, u_long heapsize,
+		u_long pagesize, const char *name, ...)
+{
+	va_list args;
+	int ret;
+
+	va_start(args, name);
+	ret = xnheap_init_va(heap, heapaddr, heapsize, pagesize, name, args);
+	va_end(args);
+
+	return ret;
+}
 EXPORT_SYMBOL_GPL(xnheap_init);
 
+/*!
+ * \fn xnheap_set_name(xnheap_t *heap,const char *name,...)
+ * \brief Overwrite the heap's name.
+ *
+ * Set the heap name that will be used in statistic outputs. This service
+ * is useful if the final name is not yet defined on xnheap_init().
+ *
+ * @param heap The address of a heap descriptor.
+ *
+ * @param name Name displayed in statistic outputs. This parameter can
+ * be a format string, in which case succeeding parameters will be used
+ * to resolve the final name.
+ *
+ * Environments:
+ *
+ * This service can be called from:
+ *
+ * - Kernel module initialization/cleanup code
+ * - Kernel-based task
+ * - User-space task
+ *
+ * Rescheduling: never.
+ */
+
+void xnheap_set_name(xnheap_t *heap, const char *name, ...)
+{
+	va_list args;
+
+	va_start(args, name);
+	vsnprintf(heap->name, sizeof(heap->name), name, args);
+	va_end(args);
+}
+EXPORT_SYMBOL_GPL(xnheap_set_name);
+
 /*! 
  * \fn void xnheap_destroy(xnheap_t *heap, void (*flushfn)(xnheap_t *heap, void *extaddr, u_long extsize, void *cookie), void *cookie)
  * \brief Destroys a memory heap.
@@ -277,6 +335,10 @@ int xnheap_destroy(xnheap_t *heap,
 	xnholder_t *holder;
 	spl_t s;
 
+	spin_lock(&heapq_lock);
+	removeq(&heapq, &heap->stat_link);
+	spin_unlock(&heapq_lock);
+
 	if (!flushfn)
 		return 0;
 
@@ -1029,6 +1091,7 @@ static void xnheap_vmclose(struct vm_area_struct *vma)
 
 	if (atomic_dec_and_test(&heap->archdep.numaps)) {
 		if (heap->archdep.release) {
+			removeq(&heapq, &heap->stat_link);
 			removeq(&kheapq, &heap->link);
 			spin_unlock(&heapq_lock);
 			__unreserve_and_free_heap(heap->archdep.heapbase,
@@ -1145,9 +1208,11 @@ static int xnheap_mmap(struct file *file, struct vm_area_struct *vma)
 	return 0;
 }
 
-int xnheap_init_mapped(xnheap_t *heap, u_long heapsize, int memflags)
+int xnheap_init_mapped(xnheap_t *heap, u_long heapsize, int memflags,
+		       const char *name, ...)
 {
 	void *heapbase;
+	va_list args;
 	int err;
 
 	/* Caller must have accounted for internal overhead. */
@@ -1161,7 +1226,9 @@ int xnheap_init_mapped(xnheap_t *heap, u_long heapsize, int memflags)
 	if (!heapbase)
 		return -ENOMEM;
 
-	err = xnheap_init(heap, heapbase, heapsize, PAGE_SIZE);
+	va_start(args, name);
+	err = xnheap_init_va(heap, heapbase, heapsize, PAGE_SIZE, name, args);
+	va_end(args);
 	if (err) {
 		__unreserve_and_free_heap(heapbase, heapsize, memflags);
 		return err;
@@ -1194,6 +1261,7 @@ int xnheap_destroy_mapped(xnheap_t *heap, void (*release)(struct xnheap *heap),
 		return -EBUSY;
 	}
 
+	removeq(&heapq, &heap->stat_link);
 	removeq(&kheapq, &heap->link); /* Prevent further mapping. */
 	spin_unlock(&heapq_lock);
 
@@ -1280,22 +1348,28 @@ static int heap_read_proc(char *page,
 			  char **start,
 			  off_t off, int count, int *eof, void *data)
 {
-	int len;
+	xnholder_t *entry;
+	xnheap_t *heap;
+	int len = 0;
 
 	if (!xnpod_active_p())
 		return -ESRCH;
 
-	len = sprintf(page, "size=%lu:used=%lu:pagesz=%lu  (main heap)\n",
-		      xnheap_usable_mem(&kheap),
-		      xnheap_used_mem(&kheap),
-		      xnheap_page_size(&kheap));
+	spin_lock(&heapq_lock);
+
+	entry = getheadq(&heapq);
+	while (entry) {
+		heap = container_of(entry, xnheap_t, stat_link);
+		len += sprintf(page + len,
+			       "size=%lu:used=%lu:pagesz=%lu  (%s)\n",
+			       xnheap_usable_mem(heap),
+			       xnheap_used_mem(heap),
+			       xnheap_page_size(heap),
+			       heap->name);
+		entry = nextq(&heapq, entry);
+	}
 
-#if CONFIG_XENO_OPT_SYS_STACKPOOLSZ > 0
-	len += sprintf(page + len, "size=%lu:used=%lu:pagesz=%lu  (stack pool)\n",
-		       xnheap_usable_mem(&kstacks),
-		       xnheap_used_mem(&kstacks),
-		       xnheap_page_size(&kstacks));
-#endif
+	spin_unlock(&heapq_lock);
 
 	len -= off;
 	if (len <= off + count)
diff --git a/ksrc/nucleus/module.c b/ksrc/nucleus/module.c
index 141276a..a0efc14 100644
--- a/ksrc/nucleus/module.c
+++ b/ksrc/nucleus/module.c
@@ -100,7 +100,7 @@ int __init __xeno_sys_init(void)
 #ifndef __XENO_SIM__
 	ret = xnheap_init_mapped(&__xnsys_global_ppd.sem_heap,
 				 CONFIG_XENO_OPT_GLOBAL_SEM_HEAPSZ * 1024,
-				 XNARCH_SHARED_HEAP_FLAGS);
+				 XNARCH_SHARED_HEAP_FLAGS, "global sem heap");
 	if (ret)
 		goto cleanup_arch;
 #endif
diff --git a/ksrc/nucleus/pod.c b/ksrc/nucleus/pod.c
index 370ad28..07fbcdb 100644
--- a/ksrc/nucleus/pod.c
+++ b/ksrc/nucleus/pod.c
@@ -384,9 +384,8 @@ int xnpod_init(void)
 	heapaddr = xnarch_alloc_host_mem(xnmod_sysheap_size);
 	if (heapaddr == NULL ||
 	    xnheap_init(&kheap, heapaddr, xnmod_sysheap_size,
-			XNHEAP_PAGE_SIZE) != 0) {
+			XNHEAP_PAGE_SIZE, "main heap") != 0)
 		return -ENOMEM;
-	}
 
 #if CONFIG_XENO_OPT_SYS_STACKPOOLSZ > 0
 	/*
@@ -404,7 +403,7 @@ int xnpod_init(void)
 	heapaddr = xnarch_alloc_stack_mem(CONFIG_XENO_OPT_SYS_STACKPOOLSZ * 1024);
 	if (heapaddr == NULL ||
 	    xnheap_init(&kstacks, heapaddr, CONFIG_XENO_OPT_SYS_STACKPOOLSZ * 1024,
-			XNHEAP_PAGE_SIZE) != 0) {
+			XNHEAP_PAGE_SIZE, "stack pool") != 0) {
 		xnheap_destroy(&kheap, &xnpod_flush_heap, NULL);
 		return -ENOMEM;
 	}
diff --git a/ksrc/nucleus/shadow.c b/ksrc/nucleus/shadow.c
index d6d1203..e8876d8 100644
--- a/ksrc/nucleus/shadow.c
+++ b/ksrc/nucleus/shadow.c
@@ -1886,7 +1886,9 @@ static void *xnshadow_sys_event(int event, void *data)
 
 		err = xnheap_init_mapped(&p->sem_heap,
 					 CONFIG_XENO_OPT_SEM_HEAPSZ * 1024,
-					 XNARCH_SHARED_HEAP_FLAGS);
+					 XNARCH_SHARED_HEAP_FLAGS,
+					 "private sem heap [%d]",
+					 current->pid);
 		if (err) {
 			xnarch_free_host_mem(p, sizeof(*p));
 			return ERR_PTR(err);
@@ -1896,6 +1898,7 @@ static void *xnshadow_sys_event(int event, void *data)
 
 	case XNSHADOW_CLIENT_DETACH:
 		p = ppd2sys((xnshadow_ppd_t *) data);
+
 		xnheap_destroy_mapped(&p->sem_heap, post_ppd_release, NULL);
 
 		return NULL;
diff --git a/ksrc/skins/native/heap.c b/ksrc/skins/native/heap.c
index 0a24735..9f3a072 100644
--- a/ksrc/skins/native/heap.c
+++ b/ksrc/skins/native/heap.c
@@ -265,7 +265,8 @@ int rt_heap_create(RT_HEAP *heap, const char *name, size_t heapsize, int mode)
 					 heapsize,
 					 ((mode & H_DMA) ? GFP_DMA : 0)
 					 | ((mode & H_NONCACHED) ?
-					    XNHEAP_GFP_NONCACHED : 0));
+					    XNHEAP_GFP_NONCACHED : 0),
+					 "rt_heap: %s", name);
 		if (err)
 			return err;
 
@@ -285,7 +286,8 @@ int rt_heap_create(RT_HEAP *heap, const char *name, size_t heapsize, int mode)
 		if (!heapmem)
 			return -ENOMEM;
 
-		err = xnheap_init(&heap->heap_base, heapmem, heapsize, XNHEAP_PAGE_SIZE);
+		err = xnheap_init(&heap->heap_base, heapmem, heapsize,
+				  XNHEAP_PAGE_SIZE, "rt_heap: %s", name);
 		if (err) {
 			xnarch_free_host_mem(heapmem, heapsize);
 			return err;
diff --git a/ksrc/skins/native/pipe.c b/ksrc/skins/native/pipe.c
index 672fa7a..18d9aa0 100644
--- a/ksrc/skins/native/pipe.c
+++ b/ksrc/skins/native/pipe.c
@@ -295,7 +295,9 @@ int rt_pipe_create(RT_PIPE *pipe, const char *name, int minor, size_t poolsize)
 			return -ENOMEM;
 
 		/* Use natural page size */
-		err = xnheap_init(&pipe->privpool, poolmem, poolsize, XNHEAP_PAGE_SIZE);
+		err = xnheap_init(&pipe->privpool, poolmem, poolsize,
+				  XNHEAP_PAGE_SIZE,
+				  "rt_pipe: %d / %s", minor, name);
 		if (err) {
 			xnarch_free_host_mem(poolmem, poolsize);
 			return err;
diff --git a/ksrc/skins/native/queue.c b/ksrc/skins/native/queue.c
index 527bde8..d2fc5c8 100644
--- a/ksrc/skins/native/queue.c
+++ b/ksrc/skins/native/queue.c
@@ -229,7 +229,8 @@ int rt_queue_create(RT_QUEUE *q,
 		err = xnheap_init_mapped(&q->bufpool,
 					 poolsize,
 					 ((mode & Q_DMA) ? GFP_DMA 
-					  : XNARCH_SHARED_HEAP_FLAGS));
+					  : XNARCH_SHARED_HEAP_FLAGS),
+					 "rt_queue: %s", name);
 		if (err)
 			return err;
 
@@ -249,7 +250,8 @@ int rt_queue_create(RT_QUEUE *q,
 		if (!poolmem)
 			return -ENOMEM;
 
-		err = xnheap_init(&q->bufpool, poolmem, poolsize, XNHEAP_PAGE_SIZE);
+		err = xnheap_init(&q->bufpool, poolmem, poolsize,
+				  XNHEAP_PAGE_SIZE, "rt_queue: %s", name);
 		if (err) {
 			xnarch_free_host_mem(poolmem, poolsize);
 			return err;
diff --git a/ksrc/skins/posix/shm.c b/ksrc/skins/posix/shm.c
index c92096a..0f0bd1a 100644
--- a/ksrc/skins/posix/shm.c
+++ b/ksrc/skins/posix/shm.c
@@ -539,7 +539,9 @@ int ftruncate(int fd, off_t len)
 			int flags = XNARCH_SHARED_HEAP_FLAGS |
 				((desc_flags & O_DIRECT) ? GFP_DMA : 0);
 
-			err = -xnheap_init_mapped(&shm->heapbase, len, flags);
+			err = -xnheap_init_mapped(&shm->heapbase, len, flags,
+						  "posix shm: %s",
+						  shm->nodebase.name);
 			if (err)
 				goto err_up;
 
diff --git a/ksrc/skins/psos+/rn.c b/ksrc/skins/psos+/rn.c
index 98f4500..3eb3ab9 100644
--- a/ksrc/skins/psos+/rn.c
+++ b/ksrc/skins/psos+/rn.c
@@ -191,7 +191,8 @@ u_long rn_create(const char *name,
 
 		rnsize = xnheap_rounded_size(rnsize, PAGE_SIZE),
 		err = xnheap_init_mapped(&rn->heapbase, rnsize, 
-					 XNARCH_SHARED_HEAP_FLAGS);
+					 XNARCH_SHARED_HEAP_FLAGS,
+					 "psosrn: %s", name);
 
 		if (err)
 			return err;
@@ -206,7 +207,8 @@ u_long rn_create(const char *name,
 		 * Caller must have accounted for overhead and
 		 * alignment since it supplies the memory space.
 		 */
-		if (xnheap_init(&rn->heapbase, rnaddr, rnsize, XNHEAP_PAGE_SIZE) != 0)
+		if (xnheap_init(&rn->heapbase, rnaddr, rnsize,
+				XNHEAP_PAGE_SIZE, "psosrn: %s", name) != 0)
 			return ERR_TINYRN;
 
 	inith(&rn->link);
diff --git a/ksrc/skins/rtai/shm.c b/ksrc/skins/rtai/shm.c
index 81de434..025535a 100644
--- a/ksrc/skins/rtai/shm.c
+++ b/ksrc/skins/rtai/shm.c
@@ -151,7 +151,8 @@ static xnshm_a_t *create_new_heap(unsigned long name, int heapsize, int suprt)
 	err = xnheap_init_mapped(p->heap,
 				 heapsize,
 				 (suprt == USE_GFP_KERNEL ? GFP_KERNEL : 0)
-				 | XNARCH_SHARED_HEAP_FLAGS);
+				 | XNARCH_SHARED_HEAP_FLAGS,
+				 "rtai heap: 0x%lx", name);
 #else /* !CONFIG_XENO_OPT_PERVASIVE */
 	{
 		void *heapmem;
@@ -164,7 +165,9 @@ static xnshm_a_t *create_new_heap(unsigned long name, int heapsize, int suprt)
 			err = -ENOMEM;
 		} else {
 
-			err = xnheap_init(p->heap, heapmem, heapsize, XNHEAP_PAGE_SIZE);
+			err = xnheap_init(p->heap, heapmem, heapsize,
+					  XNHEAP_PAGE_SIZE,
+					  "rtai heap: 0x%lx", name);
 			if (err) {
 				xnarch_free_host_mem(heapmem, heapsize);
 			}
diff --git a/ksrc/skins/vrtx/heap.c b/ksrc/skins/vrtx/heap.c
index de60792..8f8a1d5 100644
--- a/ksrc/skins/vrtx/heap.c
+++ b/ksrc/skins/vrtx/heap.c
@@ -165,7 +165,8 @@ int sc_hcreate(char *heapaddr, u_long heapsize, unsigned log2psize, int *errp)
 #ifdef CONFIG_XENO_OPT_PERVASIVE
 		heapsize = xnheap_rounded_size(heapsize, PAGE_SIZE);
 		err = xnheap_init_mapped(&heap->sysheap, heapsize, 
-					 XNARCH_SHARED_HEAP_FLAGS);
+					 XNARCH_SHARED_HEAP_FLAGS,
+					 "vrtx sysheap");
 
 		if (err) {
 			*errp = ER_MEM;
@@ -184,7 +185,8 @@ int sc_hcreate(char *heapaddr, u_long heapsize, unsigned log2psize, int *errp)
 		 * Caller must have accounted for overhead and
 		 * alignment since it supplies the memory space.
 		 */
-		err = xnheap_init(&heap->sysheap, heapaddr, heapsize, pagesize);
+		err = xnheap_init(&heap->sysheap, heapaddr, heapsize, pagesize,
+				  "vrtx sysheap");
 
 		if (err) {
 			if (err == -EINVAL)
diff --git a/ksrc/skins/vrtx/syscall.c b/ksrc/skins/vrtx/syscall.c
index 1623066..e3d2e31 100644
--- a/ksrc/skins/vrtx/syscall.c
+++ b/ksrc/skins/vrtx/syscall.c
@@ -1052,7 +1052,8 @@ static int __sc_pcreate(struct pt_regs *regs)
 	/* Block size. */
 	bsize = __xn_reg_arg3(regs);
 
-	err = xnheap_init_mapped(ptheap, ptsize, XNARCH_SHARED_HEAP_FLAGS);
+	err = xnheap_init_mapped(ptheap, ptsize, XNARCH_SHARED_HEAP_FLAGS,
+				 "vrtx ptheap");
 
 	if (err)
 		goto free_heap;

Re: [Xenomai-core] [PATCH v2 2/3] nucleus: Include all heaps in statistics

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> @@ -234,12 +239,65 @@ int xnheap_init(xnheap_t *heap,
>  
>  	appendq(&heap->extents, &extent->link);
>  
> +	vsnprintf(heap->name, sizeof(heap->name), name, args);
> +
> +	spin_lock(&heapq_lock);
> +	appendq(&heapq, &heap->stat_link);
> +	spin_unlock(&heapq_lock);

You can not use a Linux spinlock in xnheap_init and xnheap_destroy:
- this breaks the build for the simulator;
- callers of xnheap_init and xnheap_destroy are not guaranteed to run on
the root domain.

-- 
					    Gilles.

Re: [Xenomai-core] [PATCH v2 2/3] nucleus: Include all heaps in statistics

[Jan Kiszka]

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

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> @@ -234,12 +239,65 @@ int xnheap_init(xnheap_t *heap,
>>  
>>  	appendq(&heap->extents, &extent->link);
>>  
>> +	vsnprintf(heap->name, sizeof(heap->name), name, args);
>> +
>> +	spin_lock(&heapq_lock);
>> +	appendq(&heapq, &heap->stat_link);
>> +	spin_unlock(&heapq_lock);
> 
> You can not use a Linux spinlock in xnheap_init and xnheap_destroy:
> - this breaks the build for the simulator;
> - callers of xnheap_init and xnheap_destroy are not guaranteed to run on
> the root domain.

Oh, yes, unfortunately. That callers appear to be fixable, but that's
probably not worth it at this point. I will have to rewrite
heap_read_proc to break out of nklock frequently. Also not nice, but
less invasive.

Jan


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

Re: [Xenomai-core] [PATCH v2 2/3] nucleus: Include all heaps in statistics

[Philippe Gerum]

On Mon, 2009-10-19 at 21:09 +0200, Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
> > Jan Kiszka wrote:
> >> @@ -234,12 +239,65 @@ int xnheap_init(xnheap_t *heap,
> >>  
> >>  	appendq(&heap->extents, &extent->link);
> >>  
> >> +	vsnprintf(heap->name, sizeof(heap->name), name, args);
> >> +
> >> +	spin_lock(&heapq_lock);
> >> +	appendq(&heapq, &heap->stat_link);
> >> +	spin_unlock(&heapq_lock);
> > 
> > You can not use a Linux spinlock in xnheap_init and xnheap_destroy:
> > - this breaks the build for the simulator;
> > - callers of xnheap_init and xnheap_destroy are not guaranteed to run on
> > the root domain.
> 
> Oh, yes, unfortunately. That callers appear to be fixable, but that's
> probably not worth it at this point.

There is nothing to fix here. It's part of the service definition to be
able to call it from primary mode.

>  I will have to rewrite
> heap_read_proc to break out of nklock frequently. Also not nice, but
> less invasive.
> 
> Jan
> 
> _______________________________________________
> Xenomai-core mailing list
> Xenomai-core@domain.hid
> https://mail.gna.org/listinfo/xenomai-core
-- 
Philippe.

Re: [Xenomai-core] [PATCH v2 2/3] nucleus: Include all heaps in statistics

[Jan Kiszka]

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

Philippe Gerum wrote:
> On Mon, 2009-10-19 at 21:09 +0200, Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> Jan Kiszka wrote:
>>>> @@ -234,12 +239,65 @@ int xnheap_init(xnheap_t *heap,
>>>>  
>>>>  	appendq(&heap->extents, &extent->link);
>>>>  
>>>> +	vsnprintf(heap->name, sizeof(heap->name), name, args);
>>>> +
>>>> +	spin_lock(&heapq_lock);
>>>> +	appendq(&heapq, &heap->stat_link);
>>>> +	spin_unlock(&heapq_lock);
>>> You can not use a Linux spinlock in xnheap_init and xnheap_destroy:
>>> - this breaks the build for the simulator;
>>> - callers of xnheap_init and xnheap_destroy are not guaranteed to run on
>>> the root domain.
>> Oh, yes, unfortunately. That callers appear to be fixable, but that's
>> probably not worth it at this point.
> 
> There is nothing to fix here. It's part of the service definition to be
> able to call it from primary mode.

Strictly spoken not. But given that xnheap_init_mapped does not fulfill
this promise and that quite a few users have an either-or use of this
tuple, it doesn't buy us much to allow primary mode.

Jan


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

Re: [Xenomai-core] [PATCH v2 2/3] nucleus: Include all heaps in statistics

[Philippe Gerum]

On Tue, 2009-10-20 at 13:44 +0200, Jan Kiszka wrote:
> Philippe Gerum wrote:
> > On Mon, 2009-10-19 at 21:09 +0200, Jan Kiszka wrote:
> >> Gilles Chanteperdrix wrote:
> >>> Jan Kiszka wrote:
> >>>> @@ -234,12 +239,65 @@ int xnheap_init(xnheap_t *heap,
> >>>>  
> >>>>  	appendq(&heap->extents, &extent->link);
> >>>>  
> >>>> +	vsnprintf(heap->name, sizeof(heap->name), name, args);
> >>>> +
> >>>> +	spin_lock(&heapq_lock);
> >>>> +	appendq(&heapq, &heap->stat_link);
> >>>> +	spin_unlock(&heapq_lock);
> >>> You can not use a Linux spinlock in xnheap_init and xnheap_destroy:
> >>> - this breaks the build for the simulator;
> >>> - callers of xnheap_init and xnheap_destroy are not guaranteed to run on
> >>> the root domain.
> >> Oh, yes, unfortunately. That callers appear to be fixable, but that's
> >> probably not worth it at this point.
> > 
> > There is nothing to fix here. It's part of the service definition to be
> > able to call it from primary mode.
> 
> Strictly spoken not. But given that xnheap_init_mapped does not fulfill
> this promise and that quite a few users have an either-or use of this
> tuple, it doesn't buy us much to allow primary mode.

xnheap_init_mapped is derived from xnheap_init for obvious code reuse
issues, but this does not mean at all that xnheap_init should inherit
the prerequisites of xnheap_init_mapped when it comes to calling
context. Therefore, xnheap_init shall remain callable from primary
context; this is a published API out of tree code may rely on, and there
is absolutely no point in changing its requirements so far.

> 
> Jan
> 
-- 
Philippe.