Message-ID: <20160726051720.GA14875@Karyakshetra>
Date:	Tue, 26 Jul 2016 10:47:20 +0530
From:	Bhaktipriya Shridhar <bhaktipriya96@...il.com>
To:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Saurabh Karajgaonkar <skarajga@...teon.com>
Cc:	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
	Tejun Heo <tj@...nel.org>
Subject: [PATCH] usb: ftdi-elan: Remove deprecated
 create_singlethread_workqueue

The status workqueue is involved in initializing the Uxxx and polling
the Uxxx until a supported PCMCIA CardBus device is detected.
It then starts the command and respond workqueues and then loads the
module that handles the device, after which it just polls the Uxxx
looking for card ejects.

The command and respond workqueues are involved in implementing a command
sequencer for communicating with the firmware on the other side of
the FTDI chip in the Uxxx.

These workqueues have only a single work item each and hence they do not
require ordering. Also, none of the above workqueues are being used on a
memory recliam path. Hence, the singlethreaded workqueues have been
replaced with the use of system_wq.

System workqueues have been able to handle high level of concurrency
for a long time now and hence it's not required to have a singlethreaded
workqueue just to gain concurrency. Unlike a dedicated per-cpu workqueue
created with create_singlethread_workqueue(), system_wq allows multiple
work items to overlap executions even on the same CPU; however, a
per-cpu workqueue doesn't have any CPU locality or global ordering
guarantee unless the target CPU is explicitly specified and thus the
increase of local concurrency shouldn't make any difference.

The work items have been sync cancelled because they are self-requeueing
and need to wait for the in-flight work item to finish before proceeding
with destruction. Hence, they have been sync cancelled in
ftdi_status_cancel_work(), ftdi_command_cancel_work() and
ftdi_response_cancel_work(). These functions are called in
ftdi_elan_exit() to ensure that there are no pending work items while
disconnecting the driver.

Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@...il.com>
---
 drivers/usb/misc/ftdi-elan.c | 53 +++++++++-----------------------------------
 1 file changed, 10 insertions(+), 43 deletions(-)

diff --git a/drivers/usb/misc/ftdi-elan.c b/drivers/usb/misc/ftdi-elan.c
index 52c27ca..59031dc 100644
--- a/drivers/usb/misc/ftdi-elan.c
+++ b/drivers/usb/misc/ftdi-elan.c
@@ -61,9 +61,6 @@ module_param(distrust_firmware, bool, 0);
 MODULE_PARM_DESC(distrust_firmware,
 		 "true to distrust firmware power/overcurrent setup");
 extern struct platform_driver u132_platform_driver;
-static struct workqueue_struct *status_queue;
-static struct workqueue_struct *command_queue;
-static struct workqueue_struct *respond_queue;
 /*
  * ftdi_module_lock exists to protect access to global variables
  *
@@ -228,56 +225,56 @@ static void ftdi_elan_init_kref(struct usb_ftdi *ftdi)

 static void ftdi_status_requeue_work(struct usb_ftdi *ftdi, unsigned int delta)
 {
-	if (!queue_delayed_work(status_queue, &ftdi->status_work, delta))
+	if (!schedule_delayed_work(&ftdi->status_work, delta))
 		kref_put(&ftdi->kref, ftdi_elan_delete);
 }

 static void ftdi_status_queue_work(struct usb_ftdi *ftdi, unsigned int delta)
 {
-	if (queue_delayed_work(status_queue, &ftdi->status_work, delta))
+	if (schedule_delayed_work(&ftdi->status_work, delta))
 		kref_get(&ftdi->kref);
 }

 static void ftdi_status_cancel_work(struct usb_ftdi *ftdi)
 {
-	if (cancel_delayed_work(&ftdi->status_work))
+	if (cancel_delayed_work_sync(&ftdi->status_work))
 		kref_put(&ftdi->kref, ftdi_elan_delete);
 }

 static void ftdi_command_requeue_work(struct usb_ftdi *ftdi, unsigned int delta)
 {
-	if (!queue_delayed_work(command_queue, &ftdi->command_work, delta))
+	if (!schedule_delayed_work(&ftdi->command_work, delta))
 		kref_put(&ftdi->kref, ftdi_elan_delete);
 }

 static void ftdi_command_queue_work(struct usb_ftdi *ftdi, unsigned int delta)
 {
-	if (queue_delayed_work(command_queue, &ftdi->command_work, delta))
+	if (schedule_delayed_work(&ftdi->command_work, delta))
 		kref_get(&ftdi->kref);
 }

 static void ftdi_command_cancel_work(struct usb_ftdi *ftdi)
 {
-	if (cancel_delayed_work(&ftdi->command_work))
+	if (cancel_delayed_work_sync(&ftdi->command_work))
 		kref_put(&ftdi->kref, ftdi_elan_delete);
 }

 static void ftdi_response_requeue_work(struct usb_ftdi *ftdi,
 				       unsigned int delta)
 {
-	if (!queue_delayed_work(respond_queue, &ftdi->respond_work, delta))
+	if (!schedule_delayed_work(&ftdi->respond_work, delta))
 		kref_put(&ftdi->kref, ftdi_elan_delete);
 }

 static void ftdi_respond_queue_work(struct usb_ftdi *ftdi, unsigned int delta)
 {
-	if (queue_delayed_work(respond_queue, &ftdi->respond_work, delta))
+	if (schedule_delayed_work(&ftdi->respond_work, delta))
 		kref_get(&ftdi->kref);
 }

 static void ftdi_response_cancel_work(struct usb_ftdi *ftdi)
 {
-	if (cancel_delayed_work(&ftdi->respond_work))
+	if (cancel_delayed_work_sync(&ftdi->respond_work))
 		kref_put(&ftdi->kref, ftdi_elan_delete);
 }

@@ -2823,9 +2820,6 @@ static void ftdi_elan_disconnect(struct usb_interface *interface)
 			ftdi->initialized = 0;
 			ftdi->registered = 0;
 		}
-		flush_workqueue(status_queue);
-		flush_workqueue(command_queue);
-		flush_workqueue(respond_queue);
 		ftdi->disconnected += 1;
 		usb_set_intfdata(interface, NULL);
 		dev_info(&ftdi->udev->dev, "USB FTDI U132 host controller interface now disconnected\n");
@@ -2845,31 +2839,12 @@ static int __init ftdi_elan_init(void)
 	pr_info("driver %s\n", ftdi_elan_driver.name);
 	mutex_init(&ftdi_module_lock);
 	INIT_LIST_HEAD(&ftdi_static_list);
-	status_queue = create_singlethread_workqueue("ftdi-status-control");
-	if (!status_queue)
-		goto err_status_queue;
-	command_queue = create_singlethread_workqueue("ftdi-command-engine");
-	if (!command_queue)
-		goto err_command_queue;
-	respond_queue = create_singlethread_workqueue("ftdi-respond-engine");
-	if (!respond_queue)
-		goto err_respond_queue;
 	result = usb_register(&ftdi_elan_driver);
 	if (result) {
-		destroy_workqueue(status_queue);
-		destroy_workqueue(command_queue);
-		destroy_workqueue(respond_queue);
 		pr_err("usb_register failed. Error number %d\n", result);
 	}
 	return result;

-err_respond_queue:
-	destroy_workqueue(command_queue);
-err_command_queue:
-	destroy_workqueue(status_queue);
-err_status_queue:
-	pr_err("%s couldn't create workqueue\n", ftdi_elan_driver.name);
-	return -ENOMEM;
 }

 static void __exit ftdi_elan_exit(void)
@@ -2882,15 +2857,7 @@ static void __exit ftdi_elan_exit(void)
 		ftdi_status_cancel_work(ftdi);
 		ftdi_command_cancel_work(ftdi);
 		ftdi_response_cancel_work(ftdi);
-	} flush_workqueue(status_queue);
-	destroy_workqueue(status_queue);
-	status_queue = NULL;
-	flush_workqueue(command_queue);
-	destroy_workqueue(command_queue);
-	command_queue = NULL;
-	flush_workqueue(respond_queue);
-	destroy_workqueue(respond_queue);
-	respond_queue = NULL;
+	}
 }


--
2.1.4

Powered by blists - more mailing lists