netdev
[Top] [All Lists]

[PATCH] (1/11) Irda dongle module owner support

To: Jean Tourrilhes <jt@xxxxxxxxxxxxxxxxxx>, "David S. Miller" <davem@xxxxxxxxxx>
Subject: [PATCH] (1/11) Irda dongle module owner support
From: Stephen Hemminger <shemminger@xxxxxxxx>
Date: Thu, 2 Oct 2003 15:20:26 -0700
Cc: irda-users@xxxxxxxxxxxxxxxxxxxxx, netdev@xxxxxxxxxxx
Organization: Open Source Development Lab
Sender: netdev-bounce@xxxxxxxxxxx
IRDA dongle interface needed to be converted to have an owner field
to avoid races on module unload.

Eliminated the use of hashbin locking because the dongle control
code needed to do it's own locking to avoid races.   This also closed
the race between find and insert. 

The find/insert hashbin race may be a general problem with all the IRDA
hashbin stuff. IMHO the hashbin stuff should be replaced, it is full
of dead incomplete code and done better by the list macros.

diff -Nru a/include/net/irda/irda_device.h b/include/net/irda/irda_device.h
--- a/include/net/irda/irda_device.h    Thu Oct  2 14:51:18 2003
+++ b/include/net/irda/irda_device.h    Thu Oct  2 14:51:18 2003
@@ -128,6 +128,7 @@
        void (*close)(dongle_t *dongle);
        int  (*reset)(struct irda_task *task);
        int  (*change_speed)(struct irda_task *task);
+       struct module *owner;
 };
 
 /* 
diff -Nru a/net/irda/irda_device.c b/net/irda/irda_device.c
--- a/net/irda/irda_device.c    Thu Oct  2 14:51:18 2003
+++ b/net/irda/irda_device.c    Thu Oct  2 14:51:18 2003
@@ -58,6 +58,7 @@
 static void __irda_task_delete(struct irda_task *task);
 
 static hashbin_t *dongles = NULL;
+static spinlock_t dongle_lock = SPIN_LOCK_UNLOCKED;
 static hashbin_t *tasks = NULL;
 
 const char *infrared_mode[] = {
@@ -85,7 +86,7 @@
 
 int __init irda_device_init( void)
 {
-       dongles = hashbin_new(HB_LOCK);
+       dongles = hashbin_new(HB_NOLOCK);
        if (dongles == NULL) {
                printk(KERN_WARNING "IrDA: Can't allocate dongles hashbin!\n");
                return -ENOMEM;
@@ -424,25 +425,35 @@
 dongle_t *irda_device_dongle_init(struct net_device *dev, int type)
 {
        struct dongle_reg *reg;
-       dongle_t *dongle;
+       dongle_t *dongle = NULL;
 
        ASSERT(dev != NULL, return NULL;);
+       ASSERT(!in_interrupt(), return NULL;);
+
+       spin_lock(&dongle_lock);
+       reg = hashbin_find(dongles, type, NULL);
 
 #ifdef CONFIG_KMOD
-       ASSERT(!in_interrupt(), return NULL;);
        /* Try to load the module needed */
-       request_module("irda-dongle-%d", type);
+       if (!reg && capable(CAP_SYS_MODULE)) {
+               spin_unlock(&dongle_lock);
+       
+               request_module("irda-dongle-%d", type);
+               
+               spin_lock(&dongle_lock);
+               reg = hashbin_find(dongles, type, NULL);
+       }
 #endif
 
-       if (!(reg = hashbin_lock_find(dongles, type, NULL))) {
-               ERROR("IrDA: Unable to find requested dongle\n");
-               return NULL;
+       if (!reg || !try_module_get(reg->owner) ) {
+               ERROR("IrDA: Unable to find requested dongle type %x\n", type);
+               goto out;
        }
 
        /* Allocate dongle info for this instance */
        dongle = kmalloc(sizeof(dongle_t), GFP_KERNEL);
        if (!dongle)
-               return NULL;
+               goto out;
 
        memset(dongle, 0, sizeof(dongle_t));
 
@@ -450,6 +461,8 @@
        dongle->issue = reg;
        dongle->dev = dev;
 
+ out:
+       spin_unlock(&dongle_lock);
        return dongle;
 }
 
@@ -461,7 +474,7 @@
        ASSERT(dongle != NULL, return -1;);
 
        dongle->issue->close(dongle);
-
+       module_put(dongle->issue->owner);
        kfree(dongle);
 
        return 0;
@@ -472,14 +485,16 @@
  */
 int irda_device_register_dongle(struct dongle_reg *new)
 {
+       spin_lock(&dongle_lock);
        /* Check if this dongle has been registered before */
-       if (hashbin_lock_find(dongles, new->type, NULL)) {
-               MESSAGE("%s: Dongle already registered\n", __FUNCTION__);
-                return 0;
-        }
-
-       /* Insert IrDA dongle into hashbin */
-       hashbin_insert(dongles, (irda_queue_t *) new, new->type, NULL);
+       if (hashbin_find(dongles, new->type, NULL)) {
+               MESSAGE("%s: Dongle type %x already registered\n", 
+                       __FUNCTION__, new->type);
+        } else {
+               /* Insert IrDA dongle into hashbin */
+               hashbin_insert(dongles, (irda_queue_t *) new, new->type, NULL);
+       }
+       spin_unlock(&dongle_lock);
 
         return 0;
 }
@@ -494,11 +509,11 @@
 {
        struct dongle *node;
 
+       spin_lock(&dongle_lock);
        node = hashbin_remove(dongles, dongle->type, NULL);
-       if (!node) {
+       if (!node) 
                ERROR("%s: dongle not found!\n", __FUNCTION__);
-               return;
-       }
+       spin_unlock(&dongle_lock);
 }
 
 /*

<Prev in Thread] Current Thread [Next in Thread>