netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stephen Hemminger <shemminger@osdl.org>
To: Jean Tourrilhes <jt@bougret.hpl.hp.com>,
	"David S. Miller" <davem@redhat.com>
Cc: irda-users@lists.sourceforge.net, netdev@oss.sgi.com
Subject: [PATCH] (1/11) Irda dongle module owner support
Date: Thu, 2 Oct 2003 15:20:26 -0700	[thread overview]
Message-ID: <20031002152026.4bfd2c67.shemminger@osdl.org> (raw)

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);
 }
 
 /*

             reply	other threads:[~2003-10-02 22:20 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-10-02 22:20 Stephen Hemminger [this message]
2003-10-02 23:33 ` [PATCH] (1/11) Irda dongle module owner support Jean Tourrilhes
2003-10-03 17:21   ` [PATCH] (1/11) Irda dongle module owner support (revised) Stephen Hemminger
2003-10-03 17:31     ` Jean Tourrilhes
2003-10-04  6:06     ` David S. Miller
2003-10-03 13:55 ` [PATCH] (1/11) Irda dongle module owner support David S. Miller
2003-10-06 23:23 ` Jean Tourrilhes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20031002152026.4bfd2c67.shemminger@osdl.org \
    --to=shemminger@osdl.org \
    --cc=davem@redhat.com \
    --cc=irda-users@lists.sourceforge.net \
    --cc=jt@bougret.hpl.hp.com \
    --cc=netdev@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).