From 82124859696d0ac9684a32b596f032335a00fc2e Mon Sep 17 00:00:00 2001 From: Shaun Ruffell Date: Mon, 3 Jan 2011 18:25:32 +0000 Subject: [PATCH] dahdi_dynamic: dynamic drivers should not reference count themselves. Move the try_module_get/module_put calls from the various dynamic drivers into the "core" dahdi_dynamic.c file itself. This way, a reference count can always be held while calling through the function pointers. This is enabled by adding an .owner field to 'struct dahdi_dynamic_driver'. Dynamic spans are also unique in dahdi in that they require a "dahdi_cfg -s" to stop them and release the references on the modules. This is counterintuitive. This change makes sure they are reference counted just like other spans and on driver unload, if there aren't any open handles from userspace, they will take care of unwinding themselves. Signed-off-by: Shaun Ruffell Acked-by: Kinsey Moore git-svn-id: http://svn.asterisk.org/svn/dahdi/linux/trunk@9573 a0bf4364-ded3-4de4-8d8a-66a801d63aff --- drivers/dahdi/dahdi_dynamic.c | 65 +++++++++++++++++++++-------- drivers/dahdi/dahdi_dynamic_eth.c | 16 ++++--- drivers/dahdi/dahdi_dynamic_ethmf.c | 17 ++++---- drivers/dahdi/dahdi_dynamic_loc.c | 16 +++---- include/dahdi/kernel.h | 1 + 5 files changed, 69 insertions(+), 46 deletions(-) diff --git a/drivers/dahdi/dahdi_dynamic.c b/drivers/dahdi/dahdi_dynamic.c index 60c7153..a0a6c91 100644 --- a/drivers/dahdi/dahdi_dynamic.c +++ b/drivers/dahdi/dahdi_dynamic.c @@ -407,10 +407,13 @@ static void dahdi_dynamic_release(struct kref *kref) WARN_ON(test_bit(DAHDI_FLAGBIT_REGISTERED, &d->span.flags)); if (d->pvt) { - if (d->driver && d->driver->destroy) + if (d->driver && d->driver->destroy) { + __module_get(d->driver->owner); d->driver->destroy(d->pvt); - else + module_put(d->driver->owner); + } else { WARN_ON(1); + } } kfree(d->msgbuf); @@ -480,8 +483,12 @@ static int destroy_dynamic(struct dahdi_dynamic_span *dds) if (unlikely(!d)) return -EINVAL; - if (atomic_read(&d->kref.refcount) > 1) + /* We shouldn't have more than the two references at this point. If + * we do, there are probably channels that are still opened. */ + if (atomic_read(&d->kref.refcount) > 2) { + dynamic_put(d); return -EBUSY; + } dahdi_unregister(&d->span); @@ -506,6 +513,8 @@ static int dahdi_dynamic_rbsbits(struct dahdi_chan *chan, int bits) static int dahdi_dynamic_open(struct dahdi_chan *chan) { struct dahdi_dynamic *d = dynamic_from_span(chan->span); + if (!try_module_get(d->driver->owner)) + return -ENODEV; dynamic_get(d); return 0; } @@ -518,7 +527,9 @@ static int dahdi_dynamic_chanconfig(struct dahdi_chan *chan, int sigtype) static int dahdi_dynamic_close(struct dahdi_chan *chan) { struct dahdi_dynamic *d = dynamic_from_span(chan->span); + struct module *owner = d->driver->owner; dynamic_put(d); + module_put(owner); return 0; } @@ -621,8 +632,6 @@ static int create_dynamic(struct dahdi_dynamic_span *dds) } - /* Another race -- should let the module get unloaded while we - have it here */ if (!dtd) { printk(KERN_NOTICE "No such driver '%s' for dynamic span\n", dds->driver); @@ -630,23 +639,31 @@ static int create_dynamic(struct dahdi_dynamic_span *dds) return -EINVAL; } + if (!try_module_get(dtd->owner)) { + dynamic_put(d); + return -ENODEV; + } + + /* Remember the driver. We also give our reference to the driver to + * the dahdi_dyanmic here. Do not access dtd directly now. */ + d->driver = dtd; + /* Create the stuff */ - d->pvt = dtd->create(&d->span, d->addr); + d->pvt = d->driver->create(&d->span, d->addr); if (!d->pvt) { printk(KERN_NOTICE "Driver '%s' (%s) rejected address '%s'\n", dtd->name, dtd->desc, d->addr); dynamic_put(d); + module_put(dtd->owner); return -EINVAL; } - /* Remember the driver */ - d->driver = dtd; - /* Whee! We're created. Now register the span */ if (dahdi_register(&d->span, 0)) { printk(KERN_NOTICE "Unable to register span '%s'\n", d->span.name); dynamic_put(d); + module_put(dtd->owner); return -EINVAL; } @@ -660,8 +677,8 @@ static int create_dynamic(struct dahdi_dynamic_span *dds) checkmaster(); + module_put(dtd->owner); return x; - } #ifdef ENABLE_TASKLETS @@ -719,6 +736,9 @@ int dahdi_dynamic_register_driver(struct dahdi_dynamic_driver *dri) unsigned long flags; int res = 0; + if (!dri->owner) + return -EINVAL; + if (find_driver(dri->name)) { res = -1; } else { @@ -732,16 +752,22 @@ EXPORT_SYMBOL(dahdi_dynamic_register_driver); void dahdi_dynamic_unregister_driver(struct dahdi_dynamic_driver *dri) { - struct dahdi_dynamic *d; + struct dahdi_dynamic *d, *n; unsigned long flags; - spin_lock_irqsave(&driver_lock, flags); - list_del_rcu(&dri->list); - spin_unlock_irqrestore(&driver_lock, flags); - synchronize_rcu(); - - list_for_each_entry(d, &dspan_list, list) { + list_for_each_entry_safe(d, n, &dspan_list, list) { if (d->driver == dri) { + if (d->pvt) { + if (d->driver && d->driver->destroy) { + __module_get(d->driver->owner); + d->driver->destroy(d->pvt); + module_put(d->driver->owner); + d->pvt = NULL; + } else { + WARN_ON(1); + } + } + dahdi_unregister(&d->span); spin_lock_irqsave(&dspan_lock, flags); list_del_rcu(&d->list); spin_unlock_irqrestore(&dspan_lock, flags); @@ -749,6 +775,11 @@ void dahdi_dynamic_unregister_driver(struct dahdi_dynamic_driver *dri) dynamic_put(d); } } + + spin_lock_irqsave(&driver_lock, flags); + list_del_rcu(&dri->list); + spin_unlock_irqrestore(&driver_lock, flags); + synchronize_rcu(); } EXPORT_SYMBOL(dahdi_dynamic_unregister_driver); diff --git a/drivers/dahdi/dahdi_dynamic_eth.c b/drivers/dahdi/dahdi_dynamic_eth.c index bc4af21..f37458e 100644 --- a/drivers/dahdi/dahdi_dynamic_eth.c +++ b/drivers/dahdi/dahdi_dynamic_eth.c @@ -288,7 +288,6 @@ static void ztdeth_destroy(void *pvt) if (cur == z) { /* Successfully removed */ printk(KERN_INFO "TDMoE: Removed interface for %s\n", z->span->name); kfree(z); - module_put(THIS_MODULE); } } @@ -390,19 +389,18 @@ static void *ztdeth_create(struct dahdi_span *span, char *addr) z->next = zdevs; zdevs = z; spin_unlock_irqrestore(&zlock, flags); - if(!try_module_get(THIS_MODULE)) - printk(KERN_DEBUG "TDMoE: Unable to increment module use count\n"); } return z; } static struct dahdi_dynamic_driver ztd_eth = { - "eth", - "Ethernet", - ztdeth_create, - ztdeth_destroy, - ztdeth_transmit, - ztdeth_flush + .owner = THIS_MODULE, + .name = "eth", + .desc = "Ethernet", + .create = ztdeth_create, + .destroy = ztdeth_destroy, + .transmit = ztdeth_transmit, + .flush = ztdeth_flush, }; static struct notifier_block ztdeth_nblock = { diff --git a/drivers/dahdi/dahdi_dynamic_ethmf.c b/drivers/dahdi/dahdi_dynamic_ethmf.c index 7a435d4..db4cef7 100644 --- a/drivers/dahdi/dahdi_dynamic_ethmf.c +++ b/drivers/dahdi/dahdi_dynamic_ethmf.c @@ -573,7 +573,6 @@ static void ztdethmf_destroy(void *pvt) z->span->name); kfree(z->msgbuf); kfree(z); - module_put(THIS_MODULE); } else { if (z && z->span && z->span->name) { printk(KERN_ERR "Cannot find interface for %s\n", @@ -660,9 +659,6 @@ static void *ztdethmf_create(struct dahdi_span *span, char *addr) spin_unlock_irqrestore(ðmf_lock, flags); atomic_inc(&(ethmf_groups[hashaddr_to_index(z->addr_hash)].spans)); - if (!try_module_get(THIS_MODULE)) - printk(KERN_ERR "TDMoEmf: Unable to increment module use count\n"); - /* enable the timer for enabling the spans */ mod_timer(&timer, jiffies + HZ); atomic_set(&shutdown, 0); @@ -670,12 +666,13 @@ static void *ztdethmf_create(struct dahdi_span *span, char *addr) } static struct dahdi_dynamic_driver ztd_ethmf = { - "ethmf", - "Ethernet", - ztdethmf_create, - ztdethmf_destroy, - ztdethmf_transmit, - ztdethmf_flush + .owner = THIS_MODULE, + .name = "ethmf", + .desc = "Ethernet", + .create = ztdethmf_create, + .destroy = ztdethmf_destroy, + .transmit = ztdethmf_transmit, + .flush = ztdethmf_flush, }; static struct notifier_block ztdethmf_nblock = { diff --git a/drivers/dahdi/dahdi_dynamic_loc.c b/drivers/dahdi/dahdi_dynamic_loc.c index 72c8162..91d0289 100644 --- a/drivers/dahdi/dahdi_dynamic_loc.c +++ b/drivers/dahdi/dahdi_dynamic_loc.c @@ -146,7 +146,6 @@ static void dahdi_dynamic_local_destroy(void *pvt) printk(KERN_INFO "TDMoL: Removed interface for %s, key %d " "id %d\n", d->span->name, d->key, d->id); - module_put(THIS_MODULE); kfree(d); } @@ -216,9 +215,6 @@ static void *dahdi_dynamic_local_create(struct dahdi_span *span, char *address) list_add(&d->node, &dynamic_local_list); spin_unlock_irqrestore(&local_lock, flags); - if (!try_module_get(THIS_MODULE)) - printk(KERN_DEBUG "TDMoL: Unable to increment module use count\n"); - printk(KERN_INFO "TDMoL: Added new interface for %s, " "key %d id %d\n", span->name, d->key, d->id); return d; @@ -240,12 +236,12 @@ INVALID_ADDRESS: } static struct dahdi_dynamic_driver dahdi_dynamic_local = { - "loc", - "Local", - dahdi_dynamic_local_create, - dahdi_dynamic_local_destroy, - dahdi_dynamic_local_transmit, - NULL /* flush */ + .owner = THIS_MODULE, + .name = "loc", + .desc = "Local", + .create = dahdi_dynamic_local_create, + .destroy = dahdi_dynamic_local_destroy, + .transmit = dahdi_dynamic_local_transmit, }; static int __init dahdi_dynamic_local_init(void) diff --git a/include/dahdi/kernel.h b/include/dahdi/kernel.h index 863ad04..24bb1b4 100644 --- a/include/dahdi/kernel.h +++ b/include/dahdi/kernel.h @@ -1006,6 +1006,7 @@ struct dahdi_dynamic_driver { int (*flush)(void); struct list_head list; + struct module *owner; }; /*! \brief Receive a dynamic span message */