[Xenomai-core] [PATCH] shared irqs v.3

[Dmitry Adamushko <dmitry.adamushko@domain.hid>]

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

Hi,

here goes another implementation of shared irqs on the nucleus layer.
I have conducted a few tests and it seems to work. The test example is
attached.

There were 2 main issues concerning synchronization:

1)  xnintr_attach() vs. xnintr_detach() (and each of them vs. itself)

    The problem is that we can't use the "nklock" (nor any other lock + irq
off) as Gilles pointed out.

    A possible solution:

    o  something lick xnlock_get/put()

    There is no irqsave/restore -less interface of xnlock_get/put available.
For pure locking scheme
    (without touching the irqs) the concept of _preemption_ (to prevent a
thread from being preempted
    while in a locked section) must be introduced and, at the first glance,
that would be quite difficult
    since it must be consistent across all domains (if only for the primary
- that's easy).

    o  rthal_critical_enter/exit()

    This one is used currently.


2)  xnintr_attach/detach() vs. xnintr_irq_handler()

    The problem here is how to be sure that 1) the "xnintr_shirq_t" object
is valid (when dynamically allocated)
    and 2) to be safe while iterating through the handlers list.

    Currently, 1) is allowed by the static xnintr_shirq_t
xnshirqs[IPIPE_NR_IRQS]. Ok, it can be done lighter
    when a one-way-list is used instead of xnqueue_t.

    Beleive it or not, I have considered different ways to guarantee that a
passed "cookie" param is valid
    (xnintr_detach() has not deleted it) and remains to be so while the
xnintr_irq_handler() is running.
    And there are some obstacles there... I'll post them later if someone is
interested since I'm short of time now :)
    ...


There are a few ugly things in code, namely __IPIPE_NR_IRQS and definitions
of rthal_critical_enter/exit().
That code is compiled for the user-mode code also and the originals are not
available. So consider it a temp
solution for test purposes, I guess it's easily fixable.

test/shirq.c - is a test module.

SHIRQ_VECTOR must be the one used by Linux, e.g. I have used 12 that's used
by the trackball device.


--
Best regards,
Dmitry Adamushko

[-- Attachment #1.2: Type: text/html, Size: 2819 bytes --]

[-- Attachment #2: shirq-2_intr.c.patch --]
[-- Type: application/octet-stream, Size: 4907 bytes --]

--- intr-cvs.c	2005-12-02 19:56:20.000000000 +0100
+++ intr.c	2006-01-15 09:39:37.000000000 +0100
@@ -41,6 +41,62 @@ xnintr_t nkclock;
 static void xnintr_irq_handler(unsigned irq,
 			       void *cookie);
 
+typedef struct xnintr_shirq {
+
+    xnqueue_t handlers;
+    
+#ifdef CONFIG_SMP
+    atomic_counter_t active;
+#endif /* CONFIG_SMP */
+
+} xnintr_shirq_t;
+
+#define __IPIPE_NR_IRQS		224
+
+static xnintr_shirq_t xnshirqs[__IPIPE_NR_IRQS];
+
+extern unsigned long rthal_critical_enter(void (*synch)(void));
+extern void rthal_critical_exit(unsigned long flags);
+
+#ifdef CONFIG_SMP
+
+static inline void xnintr_shirq_lock(xnintr_shirq_t *shirq)
+{
+    xnarch_atomic_inc(&shirq->active);
+}
+
+static inline void xnintr_shirq_unlock(xnintr_shirq_t *shirq)
+{
+    xnarch_atomic_dec(&shirq->active);
+}
+
+static inline void xnintr_shirq_spin(xnintr_shirq_t *shirq)
+{
+    while (xnarch_atomic_get(&shirq->active))
+	cpu_relax();
+}
+
+#else /* !CONFIG_SMP */
+
+static inline void xnintr_shirq_lock(xnintr_shirq_t *shirq) {}
+static inline void xnintr_shirq_unlock(xnintr_shirq_t *shirq) {}
+static inline void xnintr_shirq_spin(xnintr_shirq_t *shirq) {}
+
+#endif /* CONFIG_SMP */
+
+int xnintr_mount(void)
+{
+    int i;
+    for (i = 0; i < __IPIPE_NR_IRQS; ++i)
+	{
+	initq(&xnshirqs[i].handlers);
+#ifdef CONFIG_SMP
+	atomic_set(&xnshirqs[i].active,0);
+#endif /* CONFIG_SMP */
+	}
+    return 0;
+}
+
 /*! 
  * \fn int xnintr_init (xnintr_t *intr,unsigned irq,xnisr_t isr,xniack_t iack,xnflags_t flags)
  * \brief Initialize an interrupt object.
@@ -130,6 +186,8 @@ int xnintr_init (xnintr_t *intr,
     intr->iack = iack;
     intr->cookie = NULL;
     intr->hits = 0;
+    intr->flags = flags;
+    inith(&intr->link);
 
     return 0;
 }
@@ -181,8 +239,7 @@ int xnintr_destroy (xnintr_t *intr)
  *
  * @param cookie A user-defined opaque value which is stored into the
  * interrupt object descriptor for further retrieval by the ISR/ISR
- * handlers.
- *
+ * handlers. *
  * @return 0 is returned on success. Otherwise, -EINVAL is returned if
  * a low-level error occurred while attaching the interrupt. -EBUSY is
  * specifically returned if the interrupt object was already attached.
@@ -204,11 +261,45 @@ int xnintr_destroy (xnintr_t *intr)
 int xnintr_attach (xnintr_t *intr,
 		   void *cookie)
 {
+    xnintr_shirq_t *shirq = &xnshirqs[intr->irq];
+    int err = 0, setup = 0, n;
+    unsigned long flags;
+
     intr->hits = 0;
     intr->cookie = cookie;
-    return xnarch_hook_irq(intr->irq,&xnintr_irq_handler,intr->iack,intr);
+
+    flags = rthal_critical_enter(NULL);
+
+    if (n = countq(&shirq->handlers))
+	{
+	xnintr_t *old = link2xnintr(getheadq(&shirq->handlers));
+
+	if (!(old->flags & intr->flags & XN_ISR_SHARED))
+	    {
+	    err = -EBUSY;
+	    goto unlock_and_exit;
+	    }
+	}
+    else
+	setup = 1;
+
+    appendq(&shirq->handlers,&intr->link);
+
+    if (setup)
+	{
+	err = xnarch_hook_irq(intr->irq,&xnintr_irq_handler,intr->iack,NULL);
+
+	if (err)
+	    removeq(&shirq->handlers,&intr->link);
+	}
+
+unlock_and_exit:
+
+    rthal_critical_exit(flags);
+    return err;
 }
 
+
 /*! 
  * \fn int xnintr_detach (xnintr_t *intr)
  * \brief Detach an interrupt object.
@@ -241,7 +332,27 @@ int xnintr_attach (xnintr_t *intr,
 int xnintr_detach (xnintr_t *intr)
 
 {
-    return xnarch_release_irq(intr->irq);
+    xnintr_shirq_t *shirq = &xnshirqs[intr->irq];
+    unsigned long flags;
+    int err = 0;
+
+    flags = rthal_critical_enter(NULL);
+
+    removeq(&shirq->handlers,&intr->link);
+
+    if (!countq(&shirq->handlers))
+	err = xnarch_release_irq(intr->irq);
+
+    rthal_critical_exit(flags);
+
+    /* The idea here is to keep the removed xnintr_t element valid as long
+       as the corresponding irq handler is running. This is one of the requirements
+       to iterate over the xnintr_shirq_t::handlers list in xnintr_irq_handler()
+       in a lockless way. */
+
+    xnintr_shirq_spin(shirq);
+
+    return err;
 }
 
 /*! 
@@ -350,17 +461,32 @@ static void xnintr_irq_handler (unsigned
 
 {
     xnsched_t *sched = xnpod_current_sched();
-    xnintr_t *intr = (xnintr_t *)cookie;
-    int s;
+    xnintr_shirq_t *shirq = &xnshirqs[irq];
+    xnholder_t *holder;
+    int s = 0;
 
     xnarch_memory_barrier();
 
     xnltt_log_event(xeno_ev_ienter,irq);
 
     ++sched->inesting;
-    s = intr->isr(intr);
+
+    xnintr_shirq_lock(shirq);
+    
+    holder = getheadq(&shirq->handlers);
+    
+    while (holder)
+	{
+	xnintr_t *intr = link2xnintr(holder);
+	holder = nextq(&shirq->handlers,holder);
+
+	s |= intr->isr(intr);
+	++intr->hits;
+	}
+
+    xnintr_shirq_unlock(shirq);
+
     --sched->inesting;
-    ++intr->hits;
 
     if (s & XN_ISR_ENABLE)
 	xnarch_enable_irq(irq);
@@ -384,6 +510,7 @@ static void xnintr_irq_handler (unsigned
     xnltt_log_event(xeno_ev_iexit,irq);
 }
 
+
 /*@}*/
 
 EXPORT_SYMBOL(xnintr_attach);

[-- Attachment #3: shirq-2_intr.h.patch --]
[-- Type: application/octet-stream, Size: 1151 bytes --]

--- intr-cvs.h	2005-11-01 23:37:45.000000000 +0100
+++ intr.h	2006-01-14 20:13:19.000000000 +0100
@@ -26,21 +26,31 @@
 #define XN_ISR_CHAINED   0x1
 #define XN_ISR_ENABLE    0x2
 
+/* Creation flags. */
+#define XN_ISR_SHARED	 0x1
+
 #if defined(__KERNEL__) || defined(__XENO_UVM__) || defined(__XENO_SIM__)
 
 struct xnintr;
 
 typedef struct xnintr {
 
-    unsigned irq;	/* !< IRQ number. */
+    xnholder_t link;
 
+#define link2xnintr(laddr) \
+((xnintr_t *)(((char *)laddr) - (int)(&((xnintr_t *)0)->link)))
+    
     xnisr_t isr;	/* !< Interrupt service routine. */
 
-    xniack_t iack;	/* !< Interrupt acknowledge routine. */
+    void *cookie;	/* !< User-defined cookie value. */
 
     unsigned long hits;	/* !< Number of receipts (since attachment). */
 
-    void *cookie;	/* !< User-defined cookie value. */
+    xnflags_t flags; 	/* !< Creation flags. */
+
+    unsigned irq;	/* !< IRQ number. */
+
+    xniack_t iack;	/* !< Interrupt acknowledge routine. */
 
 } xnintr_t;
 
@@ -50,6 +60,8 @@ extern xnintr_t nkclock;
 extern "C" {
 #endif
 
+int xnintr_mount(void);
+
 void xnintr_clock_handler(void);
 
     /* Public interface. */

[-- Attachment #4: shirq-2_module.c.patch --]
[-- Type: application/octet-stream, Size: 298 bytes --]

--- module-cvs.c	2006-01-15 09:33:10.000000000 +0100
+++ module.c	2006-01-14 20:02:51.000000000 +0100
@@ -714,6 +714,8 @@ int __init __xeno_sys_init (void)
     xnpod_init_proc();
 #endif /* CONFIG_PROC_FS */
 
+    xnintr_mount();
+
 #ifdef CONFIG_LTT
     xnltt_mount();
 #endif /* CONFIG_LTT */

[-- Attachment #5: test-shirq.tgz --]
[-- Type: application/x-gzip, Size: 1110 bytes --]