Changeset 2845930 in mainline

Timestamp:

2009-04-06T22:05:15Z (16 years ago)

Author:

Jakub Jermar <jakub@…>

Branches:

lfn, master, serial, ticket/834-toolchain-update, topic/msim-upgrade, topic/simplify-dev-export

Children:

6071a8f

Parents:

3636964

Message:

Address issues with IRQ notifications.

Location:

kernel/generic/src

Files:

• ddi/irq.c (modified) (4 diffs)
• ipc/irq.c (modified) (6 diffs)

kernel/generic/src/ddi/irq.c

@@ -102 +102 @@
 static index_t irq_ht_hash(unative_t *key);
 static bool irq_ht_compare(unative_t *key, count_t keys, link_t *item);
+static void irq_ht_remove(link_t *item);
 
 static hash_table_operations_t irq_ht_ops = {
         .hash = irq_ht_hash,
         .compare = irq_ht_compare,
-        .remove_callback = NULL         /* not used */
+        .remove_callback = irq_ht_remove,
 };
 
@@ -117 +118 @@
 static index_t irq_lin_hash(unative_t *key);
 static bool irq_lin_compare(unative_t *key, count_t keys, link_t *item);
+static void irq_lin_remove(link_t *item);
 
 static hash_table_operations_t irq_lin_ops = {
         .hash = irq_lin_hash,
         .compare = irq_lin_compare,
-        .remove_callback = NULL         /* not used */
+        .remove_callback = irq_lin_remove,
 };
 
@@ -348 +350 @@
 }
 
+/** Unlock IRQ structure after hash_table_remove().
+ *
+ * @param lnk           Link in the removed and locked IRQ structure.
+ */
+void irq_ht_remove(link_t *lnk)
+{
+        irq_t *irq = hash_table_get_instance(lnk, irq_t, link);
+        spinlock_unlock(&irq->lock);
+}
+
 /** Compute hash index for the key.
  *
@@ -407 +419 @@
 }
 
+/** Unlock IRQ structure after hash_table_remove().
+ *
+ * @param lnk           Link in the removed and locked IRQ structure.
+ */
+void irq_lin_remove(link_t *lnk)
+{
+        irq_t *irq = hash_table_get_instance(lnk, irq_t, link);
+        spinlock_unlock(&irq->lock);
+}
+
 /** @}
  */

kernel/generic/src/ipc/irq.c

@@ -143 +143 @@
         irq_code_t *code;
         irq_t *irq;
+        link_t *hlp;
         unative_t key[] = {
                 (unative_t) inr,
@@ -177 +178 @@
         ipl = interrupts_disable();
         spinlock_lock(&irq_uspace_hash_table_lock);
-        spinlock_lock(&irq->lock);
-        spinlock_lock(&box->irq_lock);
-        if (hash_table_find(&irq_uspace_hash_table, key)) {
+        hlp = hash_table_find(&irq_uspace_hash_table, key);
+        if (hlp) {
+                irq_t *hirq = hash_table_get_instance(hlp, irq_t, link);
+                /* hirq is locked */
+                spinlock_unlock(&hirq->lock);
                 code_free(code);
-                spinlock_unlock(&box->irq_lock);
-                spinlock_unlock(&irq->lock);
                 spinlock_unlock(&irq_uspace_hash_table_lock);
                 free(irq);
@@ -188 +189 @@
                 return EEXISTS;
         }
+        spinlock_lock(&irq->lock);      /* not really necessary, but paranoid */
+        spinlock_lock(&box->irq_lock);
         hash_table_insert(&irq_uspace_hash_table, key, &irq->link);
         list_append(&irq->notif_cfg.link, &box->irq_head);
@@ -223 +226 @@
         }
         irq = hash_table_get_instance(lnk, irq_t, link);
-        spinlock_lock(&irq->lock);
+        /* irq is locked */
         spinlock_lock(&box->irq_lock);
         
@@ -234 +237 @@
         list_remove(&irq->notif_cfg.link);
 
+        /*
+         * We need to drop the IRQ lock now because hash_table_remove() will try
+         * to reacquire it. That basically violates the natural locking order,
+         * but a deadlock in hash_table_remove() is prevented by the fact that
+         * we already held the IRQ lock and didn't drop the hash table lock in
+         * the meantime.
+         */
+        spinlock_unlock(&irq->lock);
+
         /* Remove the IRQ from the uspace IRQ hash table. */
         hash_table_remove(&irq_uspace_hash_table, key, 2);
         
         spinlock_unlock(&irq_uspace_hash_table_lock);
-        spinlock_unlock(&irq->lock);
         spinlock_unlock(&box->irq_lock);
         
@@ -295 +306 @@
                 code_free(irq->notif_cfg.code);
                 
+                /*
+                 * We need to drop the IRQ lock now because hash_table_remove()
+                 * will try to reacquire it. That basically violates the natural
+                 * locking order, but a deadlock in hash_table_remove() is
+                 * prevented by the fact that we already held the IRQ lock and
+                 * didn't drop the hash table lock in the meantime.
+                 */
                 spinlock_unlock(&irq->lock);