[PATCH v2] irqdev: make deliver_user_intr return void

[Junling Ma]

This sets up the stage for making deliver_user_intr a linux irq handler, which 
does not have a
return value. So we make deliver_user_intr return void, and move the check for 
dead notification
port into intr_thread. Now deliver_user_intr does not touch main_intr_queue, 
and only the server
thread and intr_thread manupulate the queue, adn the manipulations have been 
protected by a lock.

---
 device/intr.c                    | 47 ++++++++++++--------------------
 device/intr.h                    |  2 +-
 linux/dev/arch/i386/kernel/irq.c | 14 ++--------
 3 files changed, 21 insertions(+), 42 deletions(-)

diff --git a/device/intr.c b/device/intr.c
index 705dc1c6..c98e5c39 100644
--- a/device/intr.c
+++ b/device/intr.c
@@ -27,6 +27,8 @@ extern struct irqdev irqtab;
 #define main_intr_queue irqtab.intr_queue
 static boolean_t deliver_intr (int id, ipc_port_t dst_port);
 
+extern void free_irq (unsigned int irq, void *dev_id);
+
 #define PROTECT(lock, critical_section) \
 {\
   simple_lock(&lock);\
@@ -82,9 +84,8 @@ irq_acknowledge (ipc_port_t receive_port)
   return D_SUCCESS;
 }
 
-/* This function can only be used in the interrupt handler. */
-static void
-queue_intr (struct irqdev *dev, int id, user_intr_t *e)
+void
+deliver_user_intr (struct irqdev *dev, int id, user_intr_t *e)
 {
   /* Until userland has handled the IRQ in the driver, we have to keep it
    * disabled. Level-triggered interrupts would keep raising otherwise. */
@@ -99,29 +100,6 @@ queue_intr (struct irqdev *dev, int id, user_intr_t *e)
   thread_wakeup ((event_t) &intr_thread);
 }
 
-int
-deliver_user_intr (struct irqdev *dev, int id, user_intr_t *e)
-{
-  /* The reference of the port was increased
-   * when the port was installed.
-   * If the reference is 1, it means the port should
-   * have been destroyed and I destroy it now. */
-  if (e->dst_port
-      && e->dst_port->ip_references == 1)
-    {
-      printf ("irq handler [%d]: release a dead delivery port %p entry %p\n", 
id, e->dst_port, e);
-      ipc_port_release (e->dst_port);
-      e->dst_port = MACH_PORT_NULL;
-      thread_wakeup ((event_t) &intr_thread);
-      return 0;
-    }
-  else
-    {
-      queue_intr (dev, id, e);
-      return 1;
-    }
-}
-
 /* insert an interrupt entry in the queue.
  * This entry exists in the queue until
  * the corresponding interrupt port is removed.*/
@@ -163,7 +141,7 @@ out:
 void
 intr_thread (void)
 {
-  user_intr_t *e;
+  user_intr_t *e, *next;
   int id;
   ipc_port_t dst_port;
 
@@ -178,7 +156,7 @@ intr_thread (void)
       simple_lock(&irqtab.lock);
       queue_iterate (&main_intr_queue, e, user_intr_t *, chain)
        {
-         if ((!e->dst_port || e->dst_port->ip_references == 1) && e->n_unacked)
+         while (e->dst_port->ip_references == 1)
            {
              printf ("irq handler [%d]: release dead delivery %d unacked irqs 
port %p entry %p\n", e->id, e->n_unacked, e->dst_port, e);
              /* The reference of the port was increased
@@ -188,11 +166,22 @@ intr_thread (void)
               * handling can trigger, and we will cleanup later after the Linux
               * handler is cleared. */
              /* TODO: rather immediately remove from Linux handler */
+             next = (user_intr_t *)queue_next(&e->chain);
+             id = irqtab.irq[e->id];
              while (e->n_unacked)
              {
-               __enable_irq (irqtab.irq[e->id]);
+               __enable_irq (id);
                e->n_unacked--;
              }
+             ipc_port_release(e->dst_port);
+             queue_remove (&main_intr_queue, e, user_intr_t *, chain);
+             printf("irq handler [%d]: removed entry %p\n", id, e);
+             /* if e is the last handler registered for irq ID, then remove 
the linux irq handler */
+             free_irq(id, e);
+             if (e->interrupts != 0)
+               irqtab.tot_num_intr -= e->interrupts;
+             kfree ((vm_offset_t) e, sizeof (*e));
+             e = next;
            }
        }
 
diff --git a/device/intr.h b/device/intr.h
index 54ddb331..55fc60dc 100644
--- a/device/intr.h
+++ b/device/intr.h
@@ -51,7 +51,7 @@ struct irqdev {
 };
 
 extern int install_user_intr_handler (struct irqdev *dev, int id, unsigned 
long flags, user_intr_t *e);
-extern int deliver_user_intr (struct irqdev *dev, int id, user_intr_t *e);
+extern void deliver_user_intr (struct irqdev *dev, int id, user_intr_t *e);
 extern user_intr_t *insert_intr_entry (struct irqdev *dev, int id, ipc_port_t 
receive_port);
 
 void intr_thread (void);
diff --git a/linux/dev/arch/i386/kernel/irq.c b/linux/dev/arch/i386/kernel/irq.c
index 1e911f33..a01f7ab6 100644
--- a/linux/dev/arch/i386/kernel/irq.c
+++ b/linux/dev/arch/i386/kernel/irq.c
@@ -98,7 +98,6 @@ linux_intr (int irq)
 {
   struct pt_regs regs;
   struct linux_action *action = *(irq_action + irq);
-  struct linux_action **prev = &irq_action[irq];
   unsigned long flags;
 
   kstat.interrupts[irq]++;
@@ -113,18 +112,9 @@ linux_intr (int irq)
       // TODO I might need to check whether the interrupt belongs to
       // the current device. But I don't do it for now.
       if (action->user_intr)
-       {
-         if (!deliver_user_intr(&irqtab, irq, action->user_intr))
-         {
-           *prev = action->next;
-           linux_kfree(action);
-           action = *prev;
-           continue;
-         }
-       }
+        deliver_user_intr(&irqtab, irq, action->user_intr);
       else if (action->handler)
        action->handler (irq, action->dev_id, &regs);
-      prev = &action->next;
       action = action->next;
     }
 
@@ -255,7 +245,7 @@ install_user_intr_handler (struct irqdev *dev, int id, 
unsigned long flags,
   
   action->handler = NULL;
   action->next = NULL;
-  action->dev_id = NULL;
+  action->dev_id = user_intr;
   action->flags = SA_SHIRQ;
   action->user_intr = user_intr;
   
-- 
2.28.0.rc1