[Xenomai-core] Re: [patch] static buffer for timer-bheap

[Jan Kiszka <jan.kiszka@domain.hid>]

[-- Attachment #1.1: Type: text/plain, Size: 1087 bytes --]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>  > (...)
>  > +#define DECLARE_BHEAP_CONTAINER(name, sz) \
>  > +    struct { \
>  > +        bheap_t bheap; \
>  > +        bheaph_t elems[sz]; \
>  > +    } name
>  > +
> 
> Don't you mean bheaph_t *elems[sz] ?

Yep.

> 
>  > (...)
>  > +typedef struct {
>  > +    bheap_t bheap;
>  > +    bheaph_t __buffer[CONFIG_XENO_OPT_TIMER_HEAP_CAPACITY];
>  > +} xntimerq_t;
> 
> Same remark. And why not using the macro here ?

Indeed. Ugly late-night hack, improved version attached.

> 
> I would also prefer passing the bheaph_t** storage to bheap_init, and
> conserve bheap_destroy (with a callback called with the bheap_t**
> storage) in case the storage was dynamically allocated by the caller.

Do you have a concrete usage scenario in mind where this would be
required? I would rather bet that potential callers of bheap_destroy
know very well when some buffer is to be released. Looks at bit like
overkill unless someone has the real need to mix dynamically with
statically allocated bheaps.

Jan

[-- Attachment #1.2: bheap-init-v2.patch --]
[-- Type: text/plain, Size: 7218 bytes --]

Index: ksrc/nucleus/pod.c
===================================================================
--- ksrc/nucleus/pod.c	(revision 1144)
+++ ksrc/nucleus/pod.c	(working copy)
@@ -433,22 +433,11 @@ int xnpod_init(xnpod_t *pod, int minpri,
 #ifdef CONFIG_XENO_OPT_TIMING_PERIODIC
         unsigned n;
 
-        for (n = 0; n < XNTIMER_WHEELSIZE; n++) {
-            err = -xntlist_init(&pod->sched[cpu].timerwheel[n]);
-
-            if (err) {
-                xnheap_destroy(&kheap, &xnpod_flush_heap, NULL);
-                goto fail;
-            }
-        }
+        for (n = 0; n < XNTIMER_WHEELSIZE; n++)
+            xntlist_init(&pod->sched[cpu].timerwheel[n]);
 #endif /* CONFIG_XENO_OPT_TIMING_PERIODIC */
 
-        err = -xntimerq_init(&pod->sched[cpu].timerqueue);
-
-        if (err) {
-            xnheap_destroy(&kheap, &xnpod_flush_heap, NULL);
-            goto fail;
-        }
+        xntimerq_init(&pod->sched[cpu].timerqueue);
     }
 
     for (cpu = 0; cpu < nr_cpus; ++cpu) {
@@ -547,7 +536,6 @@ void xnpod_shutdown(int xtype)
 {
     xnholder_t *holder, *nholder;
     xnthread_t *thread;
-    unsigned cpu;
     spl_t s;
 
     xnlock_get_irqsave(&nklock, s);
@@ -585,16 +573,6 @@ void xnpod_shutdown(int xtype)
 
     __setbits(nkpod->status, XNPIDLE);
 
-    for (cpu = 0; cpu < xnarch_num_online_cpus(); cpu++) {
-#ifdef CONFIG_XENO_OPT_TIMING_PERIODIC
-        unsigned n;
-
-        for (n = 0; n < XNTIMER_WHEELSIZE; n++)
-            xntlist_destroy(&nkpod->sched[cpu].timerwheel[n]);
-#endif /* CONFIG_XENO_OPT_TIMING_PERIODIC */
-        xntimerq_destroy(&nkpod->sched[cpu].timerqueue);
-    }
-
     xnlock_put_irqrestore(&nklock, s);
 
 #ifdef CONFIG_XENO_OPT_REGISTRY
Index: include/nucleus/bheap.h
===================================================================
--- include/nucleus/bheap.h	(revision 1144)
+++ include/nucleus/bheap.h	(working copy)
@@ -43,9 +43,15 @@ typedef struct bheaph {
 typedef struct bheap {
     unsigned sz;
     unsigned last;
-    bheaph_t **elems;
+    bheaph_t *elems[1]; /* only padding, indexing starts at 1 */
 } bheap_t;
 
+#define DECLARE_BHEAP_CONTAINER(name, sz) \
+    struct { \
+        bheap_t bheap; \
+        bheaph_t *elems[sz]; \
+    } name
+
 static inline bheaph_t *bheap_gethead(bheap_t *heap)
 {
     if (heap->last == 1)
@@ -68,26 +74,10 @@ static inline bheaph_t *bheaph_child(bhe
     return likely(pos < heap->last) ? heap->elems[pos] : NULL;
 }
 
-static inline int bheap_init(bheap_t *heap, unsigned sz)
+static inline void bheap_init(bheap_t *heap, unsigned sz)
 {
     heap->sz = sz;
     heap->last = 1;
-    heap->elems = (bheaph_t **) xnarch_sysalloc(sz * sizeof(void *));
-
-    if (!heap->elems)
-        return ENOMEM;
-
-    /* start indexing at 1. */
-    heap->elems -= 1;
-
-    return 0;
-}
-
-static inline void bheap_destroy(bheap_t *heap)
-{    
-    xnarch_sysfree(heap->elems + 1, heap->sz * sizeof(void *));
-    heap->last = 0;
-    heap->sz = 0;
 }
 
 static inline void bheap_swap(bheap_t *heap, bheaph_t *h1, bheaph_t *h2)
@@ -115,7 +105,7 @@ static inline void bheap_down(bheap_t *h
     for (;;) {
         left = bheaph_child(heap, holder, 0);
         right = bheaph_child(heap, holder, 1);
-        
+
         if (left && right)
             minchild = bheaph_lt(left, right) ? left : right;
         else
@@ -143,10 +133,10 @@ static inline int bheap_insert(bheap_t *
 static inline int bheap_delete(bheap_t *heap, bheaph_t *holder)
 {
     bheaph_t *lasth;
-    
+
     if (heap->last == 1)
         return EINVAL;
-    
+
     --heap->last;
     if (heap->last > 1) {
         lasth = heap->elems[heap->last];
@@ -154,7 +144,7 @@ static inline int bheap_delete(bheap_t *
         bheaph_pos(lasth) = bheaph_pos(holder);
         bheap_down(heap, lasth);
     }
-   
+
     return 0;
 }
 
Index: include/nucleus/timer.h
===================================================================
--- include/nucleus/timer.h	(revision 1144)
+++ include/nucleus/timer.h	(working copy)
@@ -58,11 +58,10 @@ typedef struct {
     ((xntlholder_t *)(((char *)laddr) - offsetof(xntlholder_t, link)))
 
 } xntlholder_t;
-#define xntlholder_date(h)       ((h)->key)
-#define xntlholder_prio(h)       ((h)->prio)
-#define xntlholder_init(h)       inith(&(h)->link)
-#define xntlist_init(q)       (initq(q),0)
-#define xntlist_destroy(q)    do { } while (0)
+#define xntlholder_date(h)      ((h)->key)
+#define xntlholder_prio(h)      ((h)->prio)
+#define xntlholder_init(h)      inith(&(h)->link)
+#define xntlist_init(q)         initq(q)
 #define xntlist_head(q)                         \
     ({ xnholder_t *_h = getheadq(q);            \
         !_h ? NULL : link2tlholder(_h);         \
@@ -71,7 +70,7 @@ typedef struct {
 static inline void xntlist_insert(xnqueue_t *q, xntlholder_t *holder)
 {
     xnholder_t *p;
-    
+
     /* Insert the new timer at the proper place in the single
        queue managed when running in aperiodic mode. O(N) here,
        but users of the aperiodic mode need to pay a price for
@@ -82,7 +81,7 @@ static inline void xntlist_insert(xnqueu
             (holder->key == link2tlholder(p)->key &&
              holder->prio <= link2tlholder(p)->prio))
             break;
-        
+
     insertq(q,p->next,&holder->link);
 }
 
@@ -94,12 +93,11 @@ typedef bheaph_t xntimerh_t;
 #define xntimerh_date(h)       bheaph_key(h)
 #define xntimerh_prio(h)       bheaph_prio(h)
 #define xntimerh_init(h)       bheaph_init(h)
-typedef bheap_t xntimerq_t;
-#define xntimerq_init(q)       bheap_init((q), CONFIG_XENO_OPT_TIMER_HEAP_CAPACITY)
-#define xntimerq_destroy(q)    bheap_destroy(q)
-#define xntimerq_head(q)       bheap_gethead(q)
-#define xntimerq_insert(q, h)  bheap_insert((q),(h))
-#define xntimerq_remove(q, h)  bheap_delete((q),(h))
+typedef DECLARE_BHEAP_CONTAINER(xntimerq_t, CONFIG_XENO_OPT_TIMER_HEAP_CAPACITY);
+#define xntimerq_init(q)       bheap_init(&(q)->bheap, CONFIG_XENO_OPT_TIMER_HEAP_CAPACITY)
+#define xntimerq_head(q)       bheap_gethead(&(q)->bheap)
+#define xntimerq_insert(q, h)  bheap_insert(&(q)->bheap,(h))
+#define xntimerq_remove(q, h)  bheap_delete(&(q)->bheap,(h))
 
 #else /* CONFIG_XENO_OPT_TIMER_LIST */
 typedef xntlholder_t xntimerh_t;
Index: ChangeLog
===================================================================
--- ChangeLog	(revision 1144)
+++ ChangeLog	(working copy)
@@ -1,3 +1,9 @@
+2006-06-02  Jan Kiszka  <jan.kiszka@domain.hid>
+
+	* include/nucleus/{bheap.h,timer.h}, ksrc/nucleus/pod.c:
+	Move timer heap into xnpod_t structure, simplify pod init and
+	cleanup.
+
 2006-05-23  Gilles Chanteperdrix  <gilles.chanteperdrix@xenomai.org>
 
 	* ksrc/arch/i386/nmi.c: Fix alignement for gcc-4.1.
@@ -14,7 +20,7 @@
 
 	* ksrc/arch/arm/patches: Upgrade to 2.6.1{4,5}-1.3-04.
 
-2006-05-19  Jan Kiszka  <jan.kiszka@domain.hid>
+2006-05-19  Jan Kiszka  <jan.kiszka@domain.hid>
 
 	* src/testsuite/latency/latency.c: Add pid to registered names
 	allowing multiple instances of latency to run. Add -c switch to

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