From: Tim Sell <Timothy.Sell@xxxxxxxxxx>
visornic_remove() is called to logically detach the visornic driver from a
visorbus-supplied device, which can happen either just prior to a
visorbus-supplied device disappearing, or as a result of an rmmod of
visornic. Prior to this patch, logic was missing to properly clean up for
this removal, which was fixed via the following changes:
* A going_away flag is now used to interlock between device destruction and
workqueue operations, protected by priv_lock. I.e., setting
going_away=true under lock guarantees that no new work items can get
queued to the work queues. going_away=true also short-circuits other
operations to enable device destruction to proceed.
* Missing clean-up operations for the workqueues, netdev, debugfs entries,
and the worker thread were added.
* Memory referenced from the visornic private devdata struct is now freed
as part of devdata destruction.
Signed-off-by: Tim Sell <Timothy.Sell@xxxxxxxxxx>
Signed-off-by: Benjamin Romer <benjamin.romer@xxxxxxxxxx>
---
drivers/staging/unisys/visornic/visornic_main.c | 65 ++++++++++++++++++++++++-
1 file changed, 63 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/unisys/visornic/visornic_main.c b/drivers/staging/unisys/visornic/visornic_main.c
index 2ff7f2f..ba46053 100644
--- a/drivers/staging/unisys/visornic/visornic_main.c
+++ b/drivers/staging/unisys/visornic/visornic_main.c
@@ -162,6 +162,7 @@ struct visornic_devdata {
*/
bool server_down; /* IOPART is down */
bool server_change_state; /* Processing SERVER_CHANGESTATE msg */
+ bool going_away; /* device is being torn down */
struct dentry *eth_debugfs_dir;
struct visor_thread_info threadinfo;
u64 interrupts_rcvd;
@@ -568,7 +569,17 @@ static int
visornic_serverdown(struct visornic_devdata *devdata,
visorbus_state_complete_func complete_func)
{
+ unsigned long flags;
+
+ spin_lock_irqsave(&devdata->priv_lock, flags);
if (!devdata->server_down && !devdata->server_change_state) {
+ if (devdata->going_away) {
+ spin_unlock_irqrestore(&devdata->priv_lock, flags);
+ dev_dbg(&devdata->dev->device,
+ "%s aborting because device removal pending\n",
+ __func__);
+ return -ENODEV;
+ }
devdata->server_change_state = true;
devdata->server_down_complete_func = complete_func;
queue_work(visornic_serverdown_workqueue,
@@ -576,8 +587,10 @@ visornic_serverdown(struct visornic_devdata *devdata,
} else if (devdata->server_change_state) {
dev_dbg(&devdata->dev->device, "%s changing state\n",
__func__);
+ spin_unlock_irqrestore(&devdata->priv_lock, flags);
return -EINVAL;
}
+ spin_unlock_irqrestore(&devdata->priv_lock, flags);
return 0;
}
@@ -1236,6 +1249,14 @@ visornic_xmit_timeout(struct net_device *netdev)
unsigned long flags;
spin_lock_irqsave(&devdata->priv_lock, flags);
+ if (devdata->going_away) {
+ spin_unlock_irqrestore(&devdata->priv_lock, flags);
+ dev_dbg(&devdata->dev->device,
+ "%s aborting because device removal pending\n",
+ __func__);
+ return;
+ }
+
/* Ensure that a ServerDown message hasn't been received */
if (!devdata->enabled ||
(devdata->server_down && !devdata->server_change_state)) {
@@ -1244,9 +1265,8 @@ visornic_xmit_timeout(struct net_device *netdev)
spin_unlock_irqrestore(&devdata->priv_lock, flags);
return;
}
- spin_unlock_irqrestore(&devdata->priv_lock, flags);
-
queue_work(visornic_timeout_reset_workqueue, &devdata->timeout_reset);
+ spin_unlock_irqrestore(&devdata->priv_lock, flags);
}
/**
@@ -1614,6 +1634,9 @@ static void devdata_release(struct kref *mykref)
spin_lock(&lock_all_devices);
list_del(&devdata->list_all);
spin_unlock(&lock_all_devices);
+ kfree(devdata->rcvbuf);
+ kfree(devdata->cmdrsp_rcv);
+ kfree(devdata->xmit_cmdrsp);
kfree(devdata);
}
@@ -2025,13 +2048,51 @@ static void host_side_disappeared(struct visornic_devdata *devdata)
static void visornic_remove(struct visor_device *dev)
{
struct visornic_devdata *devdata = dev_get_drvdata(&dev->device);
+ struct net_device *netdev;
+ unsigned long flags;
if (!devdata) {
dev_err(&dev->device, "%s no devdata\n", __func__);
return;
}
+ spin_lock_irqsave(&devdata->priv_lock, flags);
+ if (devdata->going_away) {
+ spin_unlock_irqrestore(&devdata->priv_lock, flags);
+ dev_err(&dev->device, "%s already being removed\n", __func__);
+ return;
+ }
+ devdata->going_away = true;
+ spin_unlock_irqrestore(&devdata->priv_lock, flags);
+ netdev = devdata->netdev;
+ if (!netdev) {
+ dev_err(&dev->device, "%s not net device\n", __func__);
+ return;
+ }
+
+ /* going_away prevents new items being added to the workqueues */
+ flush_workqueue(visornic_serverdown_workqueue);
+ flush_workqueue(visornic_timeout_reset_workqueue);
+
+ debugfs_remove_recursive(devdata->eth_debugfs_dir);
+
+ unregister_netdev(netdev); /* this will call visornic_close() */
+
+ /* this had to wait until last because visornic_close() /
+ * visornic_disable_with_timeout() polls waiting for state that is
+ * only updated by the thread
+ */
+ if (devdata->threadinfo.id) {
+ visor_thread_stop(&devdata->threadinfo);
+ if (devdata->threadinfo.id) {
+ dev_err(&dev->device, "%s cannot stop worker thread\n",
+ __func__);
+ return;
+ }
+ }
+
dev_set_drvdata(&dev->device, NULL);
host_side_disappeared(devdata);
+ free_netdev(netdev);
kref_put(&devdata->kref, devdata_release);
}
--
2.1.4
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
