[PATCH] Move user_intr handling to mach side

[Junling Ma]

1. make deliver_intr a linux irq handler, and do not let it touch the 
main_intr_queue. the queue
   handling is solely in intr_thread (removing dead entries) and 
insert_intr_entry (insertion).
2. remove user_intr from struct linux_action and its handling on linux side.

---
 device/intr.c                    | 156 +++++++++++++------------------
 device/intr.h                    |   1 -
 linux/dev/arch/i386/kernel/irq.c |  73 +--------------
 3 files changed, 65 insertions(+), 165 deletions(-)

diff --git a/device/intr.c b/device/intr.c
index 705dc1c6..3962d7aa 100644
--- a/device/intr.c
+++ b/device/intr.c
@@ -23,10 +23,24 @@
 
 #ifndef MACH_XEN
 
+#define SA_SHIRQ 0x04000000
+
 extern struct irqdev irqtab;
 #define main_intr_queue irqtab.intr_queue
 static boolean_t deliver_intr (int id, ipc_port_t dst_port);
 
+struct pt_regs;
+extern int request_irq (unsigned int irq, void (*handler) (int, void *, struct 
pt_regs *),
+            unsigned long flags, const char *device, void *dev_id);
+extern void free_irq (unsigned int irq, void *dev_id);
+
+#define SPLHIGH(critical_section) \
+{\
+  spl_t s = splhigh();\
+  critical_section;\
+  splx(s);\
+}
+
 #define PROTECT(lock, critical_section) \
 {\
   simple_lock(&lock);\
@@ -82,88 +96,52 @@ 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 (int id, void *dev_id, struct pt_regs *regs)
 {
-  /* Until userland has handled the IRQ in the driver, we have to keep it
-   * disabled. Level-triggered interrupts would keep raising otherwise. */
-  __disable_irq (dev->irq[id]);
-
-  spl_t s = splhigh ();
-  e->n_unacked++;
-  e->interrupts++;
-  dev->tot_num_intr++;
-  splx (s);
-
+  SPLHIGH({
+    user_intr_t *e = dev_id;
+    /* Until userland has handled the IRQ in the driver, we have to keep it
+     * disabled. Level-triggered interrupts would keep raising otherwise. */
+    __disable_irq (irqtab.irq[id]);
+    e->n_unacked++;
+    e->interrupts++;
+    irqtab.tot_num_intr++;
+  });
   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.*/
 user_intr_t *
 insert_intr_entry (struct irqdev *dev, int id, ipc_port_t dst_port)
 {
-  user_intr_t *e, *new, *ret;
-  int free = 0;
-
-  new = (user_intr_t *) kalloc (sizeof (*new));
-  if (new == NULL)
-    return NULL;
-
   /* check whether the intr entry has been in the queue. */
-  spl_t s = splhigh ();
-  e = search_intr (dev, dst_port);
+  user_intr_t *e = search_intr (dev, dst_port);
   if (e)
     {
       printf ("the interrupt entry for irq[%d] and port %p has already been 
inserted\n", id, dst_port);
-      free = 1;
-      ret = NULL;
-      goto out;
+      return NULL;
     }
-  printf("irq handler [%d]: new delivery port %p entry %p\n", id, dst_port, 
new);
-  ret = new;
-  new->id = id;
-  new->dst_port = dst_port;
-  new->interrupts = 0;
-  new->n_unacked = 0;
-
-  PROTECT(dev->lock, queue_enter (&dev->intr_queue, new, user_intr_t *, 
chain));
-out:
-  splx (s);
-  if (free)
-    kfree ((vm_offset_t) new, sizeof (*new));
-  return ret;
+
+  e = (user_intr_t *) kalloc (sizeof (*e));
+  if (e == NULL)
+    return NULL;
+  e->id = id;
+  e->dst_port = dst_port;
+  e->interrupts = 0;
+  e->n_unacked = 0;
+  /* we do not need to disable irq here, because the irq handler does not 
touch the queue now. */
+  PROTECT(dev->lock, queue_enter (&dev->intr_queue, e, user_intr_t *, chain));
+  printf("irq handler [%d]: new delivery port %p entry %p\n", id, dst_port, e);
+  return e;
 }
 
 void
 intr_thread (void)
 {
-  user_intr_t *e;
+  user_intr_t *e, *next;
   int id;
   ipc_port_t dst_port;
 
@@ -178,8 +156,11 @@ 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)
+         /* e cannot be NULL, as we check against it when installing the 
handler */
+         assert(e != NULL);
+         while (e->dst_port->ip_references == 1)
            {
+             next = (user_intr_t *)queue_next(&e->chain);
              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
               * when the port was installed.
@@ -193,25 +174,22 @@ intr_thread (void)
                __enable_irq (irqtab.irq[e->id]);
                e->n_unacked--;
              }
+             ipc_port_release(e->dst_port);
+             assert (!queue_empty (&main_intr_queue));
+             queue_remove (&main_intr_queue, e, user_intr_t *, chain);
+             printf("irq handler [%d]: removed entry %p\n", e->id, e);
+             /* if e is the last handler registered for irq ID, then remove 
the linux irq handler */
+             free_irq(e->id, e);
+             kfree ((vm_offset_t) e, sizeof (*e));
+             e = next;
            }
        }
 
       /* Now check for interrupts */
       while (irqtab.tot_num_intr)
        {
-         int del = 0;
-
          queue_iterate (&main_intr_queue, e, user_intr_t *, chain)
            {
-             /* if an entry doesn't have dest port,
-              * we should remove it. */
-             if (e->dst_port == MACH_PORT_NULL)
-               {
-                 clear_wait (current_thread (), 0, 0);
-                 del = 1;
-                 break;
-               }
-
              if (e->interrupts)
                {
                  clear_wait (current_thread (), 0, 0);
@@ -225,24 +203,6 @@ intr_thread (void)
                  s = splhigh ();
                }
            }
-
-         /* remove the entry without dest port from the queue and free it. */
-         if (del)
-           {
-             assert (!queue_empty (&main_intr_queue));
-             queue_remove (&main_intr_queue, e, user_intr_t *, chain);
-             if (e->n_unacked)
-               printf("irq handler [%d]: still %d unacked irqs in entry %p\n", 
e->id, e->n_unacked, e);
-             while (e->n_unacked)
-             {
-               __enable_irq (irqtab.irq[e->id]);
-               e->n_unacked--;
-             }
-             printf("irq handler [%d]: removed entry %p\n", e->id, e);
-             splx (s);
-             kfree ((vm_offset_t) e, sizeof (*e));
-             s = splhigh ();
-           }
        }
       simple_unlock(&irqtab.lock);
       splx (s);
@@ -294,4 +254,16 @@ deliver_intr (int id, ipc_port_t dst_port)
   return TRUE;
 }
 
+int
+install_user_intr_handler (struct irqdev *dev, int id, unsigned long flags,
+                         user_intr_t *user_intr)
+{
+  if (id >= NINTR || user_intr == NULL)
+    return D_INVALID_OPERATION;
+  unsigned int irq = dev->irq[id];
+  if (irq >= NINTR)
+    return D_INVALID_OPERATION;
+  return request_irq (id, deliver_user_intr, SA_SHIRQ, NULL, user_intr);
+}
+
 #endif /* MACH_XEN */
diff --git a/device/intr.h b/device/intr.h
index 54ddb331..b1c09e6c 100644
--- a/device/intr.h
+++ b/device/intr.h
@@ -51,7 +51,6 @@ 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 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..5879165f 100644
--- a/linux/dev/arch/i386/kernel/irq.c
+++ b/linux/dev/arch/i386/kernel/irq.c
@@ -78,7 +78,6 @@ struct linux_action
   void *dev_id;
   struct linux_action *next;
   unsigned long flags;
-  user_intr_t *user_intr;
 };
 
 static struct linux_action *irq_action[16] =
@@ -98,7 +97,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]++;
@@ -110,32 +108,11 @@ linux_intr (int irq)
 
   while (action)
     {
-      // 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;
-         }
-       }
-      else if (action->handler)
+      if (action->handler)
        action->handler (irq, action->dev_id, &regs);
-      prev = &action->next;
       action = action->next;
     }
 
-  if (!irq_action[irq])
-    {
-      /* No handler any more, disable interrupt */
-      mask_irq (irq);
-      ivect[irq] = intnull;
-      iunit[irq] = irq;
-    }
-
   restore_flags (flags);
 
   intr_count--;
@@ -219,53 +196,6 @@ setup_x86_irq (int irq, struct linux_action *new)
   return 0;
 }
 
-int
-install_user_intr_handler (struct irqdev *dev, int id, unsigned long flags,
-                         user_intr_t *user_intr)
-{
-  struct linux_action *action;
-  struct linux_action *old;
-  int retval;
-
-  unsigned int irq = dev->irq[id];
-
-  assert (irq < 16);
-
-  /* Test whether the irq handler has been set */
-  // TODO I need to protect the array when iterating it.
-  old = irq_action[irq];
-  while (old)
-    {
-      if (old->user_intr && old->user_intr->dst_port == user_intr->dst_port)
-       {
-         printk ("The interrupt handler has already been installed on line 
%d", irq);
-         return linux_to_mach_error (-EAGAIN);
-       }
-      old = old->next;
-    }
-
-  /*
-   * Hmm... Should I use `kalloc()' ?
-   * By OKUJI Yoshinori.
-   */
-  action = (struct linux_action *)
-    linux_kmalloc (sizeof (struct linux_action), GFP_KERNEL);
-  if (action == NULL)
-    return linux_to_mach_error (-ENOMEM);
-  
-  action->handler = NULL;
-  action->next = NULL;
-  action->dev_id = NULL;
-  action->flags = SA_SHIRQ;
-  action->user_intr = user_intr;
-  
-  retval = setup_x86_irq (irq, action);
-  if (retval)
-    linux_kfree (action);
-  
-  return linux_to_mach_error (retval);
-}
-
 /*
  * Attach a handler to an IRQ.
  */
@@ -294,7 +224,6 @@ request_irq (unsigned int irq, void (*handler) (int, void 
*, struct pt_regs *),
   action->next = NULL;
   action->dev_id = dev_id;
   action->flags = flags;
-  action->user_intr = NULL;
   
   retval = setup_x86_irq (irq, action);
   if (retval)
-- 
2.28.0.rc1