Commit 46df8226 authored by Tim Sell's avatar Tim Sell Committed by Greg Kroah-Hartman
Browse files

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: default avatarTim Sell <Timothy.Sell@unisys.com>
Signed-off-by: default avatarBenjamin Romer <benjamin.romer@unisys.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent fa15d6d3

drivers/staging/unisys/visornic/visornic_main.c

+63 −2
Original line number Diff line number Diff line
@@ -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);

}