[PATCH 1/2] usbatm: Increment module refcount when atm device is opened.

[PATCH 1/2] usbatm: Increment module refcount when atm device is opened.

[Simon Arlott]

[-- Attachment #1: Type: text/plain, Size: 4201 bytes --]

Increment usbatm driver module refcount when atm device is opened, this prevents the driver for the device being removed if it's in use. (I continue to allow removing the driver without unplugging the device if it's not being used). No problems occur if the atm device is open while the device is unplugged.

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
 drivers/usb/atm/cxacru.c     |    1 +
 drivers/usb/atm/speedtch.c   |    1 +
 drivers/usb/atm/ueagle-atm.c |    1 +
 drivers/usb/atm/usbatm.c     |   14 ++++++++++++--
 drivers/usb/atm/usbatm.h     |    1 +
 drivers/usb/atm/xusbatm.c    |    1 +
 6 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/atm/cxacru.c b/drivers/usb/atm/cxacru.c
index 1049e34..424b0f4 100644
--- a/drivers/usb/atm/cxacru.c
+++ b/drivers/usb/atm/cxacru.c
@@ -980,6 +980,7 @@ static const struct usb_device_id cxacru
 MODULE_DEVICE_TABLE(usb, cxacru_usb_ids);
 
 static struct usbatm_driver cxacru_driver = {
+	.owner		= THIS_MODULE,
 	.driver_name	= cxacru_driver_name,
 	.bind		= cxacru_bind,
 	.heavy_init	= cxacru_heavy_init,
diff --git a/drivers/usb/atm/speedtch.c b/drivers/usb/atm/speedtch.c
index 638b800..5d6a0eb 100644
--- a/drivers/usb/atm/speedtch.c
+++ b/drivers/usb/atm/speedtch.c
@@ -924,6 +924,7 @@ static void speedtch_unbind(struct usbat
 ***********/
 
 static struct usbatm_driver speedtch_usbatm_driver = {
+	.owner		= THIS_MODULE,
 	.driver_name	= speedtch_driver_name,
 	.bind		= speedtch_bind,
 	.heavy_init	= speedtch_heavy_init,
diff --git a/drivers/usb/atm/ueagle-atm.c b/drivers/usb/atm/ueagle-atm.c
index dae4ef1..e32c93f 100644
--- a/drivers/usb/atm/ueagle-atm.c
+++ b/drivers/usb/atm/ueagle-atm.c
@@ -1735,6 +1735,7 @@ static void uea_unbind(struct usbatm_dat
 }
 
 static struct usbatm_driver uea_usbatm_driver = {
+	.owner = THIS_MODULE,
 	.driver_name = "ueagle-atm",
 	.bind = uea_bind,
 	.atm_start = uea_atm_open,
diff --git a/drivers/usb/atm/usbatm.c b/drivers/usb/atm/usbatm.c
index d91ed11..83cea01 100644
--- a/drivers/usb/atm/usbatm.c
+++ b/drivers/usb/atm/usbatm.c
@@ -828,6 +828,12 @@ static int usbatm_atm_open(struct atm_vc
 
 	mutex_lock(&instance->serialize);	/* vs self, usbatm_atm_close, usbatm_usb_disconnect */
 
+ 	if (!try_module_get(instance->driver->owner)) {
+ 		atm_dbg(instance, "%s: try_module_get failed\n", __func__);
+ 		ret = -ENODEV;
+ 		goto fail;
+ 	}
+
 	if (instance->disconnected) {
 		atm_dbg(instance, "%s: disconnected!\n", __func__);
 		ret = -ENODEV;
@@ -854,7 +860,7 @@ static int usbatm_atm_open(struct atm_vc
 	if (!new->sarb) {
 		atm_err(instance, "%s: no memory for SAR buffer!\n", __func__);
 		ret = -ENOMEM;
-		goto fail;
+		goto fail_free;
 	}
 
 	vcc->dev_data = new;
@@ -876,8 +882,10 @@ static int usbatm_atm_open(struct atm_vc
 
 	return 0;
 
-fail:
+fail_free:
 	kfree(new);
+fail:
+	module_put(instance->driver->owner);
 	mutex_unlock(&instance->serialize);
 	return ret;
 }
@@ -922,6 +930,8 @@ static void usbatm_atm_close(struct atm_
 	clear_bit(ATM_VF_PARTIAL, &vcc->flags);
 	clear_bit(ATM_VF_ADDR, &vcc->flags);
 
+	module_put(instance->driver->owner);
+
 	mutex_unlock(&instance->serialize);
 
 	atm_dbg(instance, "%s successful\n", __func__);
diff --git a/drivers/usb/atm/usbatm.h b/drivers/usb/atm/usbatm.h
index d3c0ee4..4a172a9 100644
--- a/drivers/usb/atm/usbatm.h
+++ b/drivers/usb/atm/usbatm.h
@@ -103,6 +103,7 @@ struct usbatm_data;
 */
 
 struct usbatm_driver {
+	struct module *owner;
 	const char *driver_name;
 
 	/* init device ... can sleep, or cause probe() failure */
diff --git a/drivers/usb/atm/xusbatm.c b/drivers/usb/atm/xusbatm.c
index 70125c6..8c425d7 100644
--- a/drivers/usb/atm/xusbatm.c
+++ b/drivers/usb/atm/xusbatm.c
@@ -205,6 +205,7 @@ static int __init xusbatm_init(void)
 		xusbatm_usb_ids[i].idVendor	= vendor[i];
 		xusbatm_usb_ids[i].idProduct	= product[i];
 
+		xusbatm_drivers[i].owner	= THIS_MODULE;
 		xusbatm_drivers[i].driver_name	= xusbatm_driver_name;
 		xusbatm_drivers[i].bind		= xusbatm_bind;
 		xusbatm_drivers[i].unbind	= xusbatm_unbind;
-- 
1.4.3.1


-- 
Simon Arlott


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 829 bytes --]

Re: [PATCH 1/2] usbatm: Increment module refcount when atm device is opened.

[Duncan Sands]

On Wednesday 21 February 2007 22:36:14 Simon Arlott wrote:
> Increment usbatm driver module refcount when atm device is opened,
> this prevents the driver for the device being removed if it's in use.
> (I continue to allow removing the driver without unplugging the device
> if it's not being used). No problems occur if the atm device is open
> while the device is unplugged.    

What for?  Did you see any problems with the way it works right now?
How it works now is that if the module is unloaded while the device is
in use, then the device is deregistered from the USB and ATM layers
before the module unload completes.  Thus there should be no problem
unloading the device at any moment.  I'm guessing that you want this
because of the better proc support you would like to add, which adds
an extra callback into the module.  This too should cause no problems
as long as the appropriate tweaks are made to the shutdown code.  I
plan to make those adjustments, but I didn't find time yet - sorry.

So this is a nack.

Best wishes,

Duncan.

Re: [PATCH 1/2] usbatm: Increment module refcount when atm device is opened.

[Simon Arlott]

[-- Attachment #1: Type: text/plain, Size: 1865 bytes --]

On 22/02/07 09:48, Duncan Sands wrote:
> What for?  Did you see any problems with the way it works right now?

It didn't make sense to be able to unload a module while it's actually in use, how does this work with automatic module loading/unloading (although I can't see how it's going to automatically load one of the sub drivers...)?


> How it works now is that if the module is unloaded while the device is
> in use, then the device is deregistered from the USB and ATM layers
> before the module unload completes.  Thus there should be no problem
> unloading the device at any moment.  
Ok.


> I'm guessing that you want this because of the better proc support you
> would like to add, which adds an extra callback into the module.

Actually it was partly because of the urb warning messages on unload... which the change doesn't even affect.


> This too should cause no problems as long as the appropriate tweaks are
> made to the shutdown code. I plan to make those adjustments, but I didn't
> find time yet - sorry.

I intend to export all the data via sysfs instead, since the current method of using proc (requiring several large enough read()s and no seeking) isn't very good and I can't see any way to improve it without caching the whole output from file open to close which atm doesn't support.

I was thinking that the MAC address/AAL stats should be removed from the proc output since they're already available from the atm devices proc file?


> PS: I plan to work on the drivers this weekend.

I don't expect to require any changes to usbatm to support sysfs, since the attributes should go with the usb device itself (and not the atm device). Perhaps there could be symlinks in each direction between the usb device and the atm device (like dvb/v4l do, e.g. atm:cxacru0 in the usb device)?

-- 
Simon Arlott


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 829 bytes --]