staging: unisys: visornic: correctly clean up device on removal

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@unisys.com>
Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/staging/unisys/visornic/visornic_main.c b/drivers/staging/unisys/visornic/visornic_main.c
index 2ff7f2f..ba46053 100644 (file)
--- 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);
 }