diff mbox

spi: spi-topcliff-pch: Remove deprecated create_singlethread_workqueue

Message ID 20160702085334.GA28440@Karyakshetra (mailing list archive)
State New, archived
Headers show

Commit Message

Bhaktipriya Shridhar July 2, 2016, 8:53 a.m. UTC 
The workqueue "wk" serves as a queue for carrying out execution
of requests. It has a single work item(&drv_data->work) and hence doesn't
require ordering. Also, it is not being used on a memory reclaim path.
Hence, the singlethreaded workqueue has 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.

Work item has been flushed in pch_spi_free_resources() to ensure that
there are no pending tasks while disconnecting the driver.

Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>
---
 drivers/spi/spi-topcliff-pch.c | 25 +++----------------------
 1 file changed, 3 insertions(+), 22 deletions(-)

--
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Tejun Heo July 2, 2016, 11:42 a.m. UTC | #1 
On Sat, Jul 02, 2016 at 02:23:34PM +0530, Bhaktipriya Shridhar wrote:
> The workqueue "wk" serves as a queue for carrying out execution
> of requests. It has a single work item(&drv_data->work) and hence doesn't
> require ordering. Also, it is not being used on a memory reclaim path.
> Hence, the singlethreaded workqueue has 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.
> 
> Work item has been flushed in pch_spi_free_resources() to ensure that
> there are no pending tasks while disconnecting the driver.
> 
> Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>

Acked-by: Tejun Heo <tj@kernel.org>

Thanks.
kernel test robot July 2, 2016, 12:24 p.m. UTC | #2 
Hi,

[auto build test WARNING on spi/for-next]
[also build test WARNING on v4.7-rc5 next-20160701]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Bhaktipriya-Shridhar/spi-spi-topcliff-pch-Remove-deprecated-create_singlethread_workqueue/20160702-191734
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 4.9.0
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=ia64 

All warnings (new ones prefixed by >>):

   drivers/spi/spi-topcliff-pch.c: In function 'pch_spi_get_resources':
>> drivers/spi/spi-topcliff-pch.c:1283:1: warning: label 'err_return' defined but not used [-Wunused-label]
    err_return:
    ^

vim +/err_return +1283 drivers/spi/spi-topcliff-pch.c

e8b17b5b drivers/spi/spi_topcliff_pch.c Masayuki Ohtake 2010-10-08  1267  
f016aeb6 drivers/spi/spi-topcliff-pch.c Tomoya MORINAGA 2011-06-07  1268  static int pch_spi_get_resources(struct pch_spi_board_data *board_dat,
f016aeb6 drivers/spi/spi-topcliff-pch.c Tomoya MORINAGA 2011-06-07  1269  				 struct pch_spi_data *data)
e8b17b5b drivers/spi/spi_topcliff_pch.c Masayuki Ohtake 2010-10-08  1270  {
f016aeb6 drivers/spi/spi-topcliff-pch.c Tomoya MORINAGA 2011-06-07  1271  	int retval = 0;
f016aeb6 drivers/spi/spi-topcliff-pch.c Tomoya MORINAGA 2011-06-07  1272  
e8b17b5b drivers/spi/spi_topcliff_pch.c Masayuki Ohtake 2010-10-08  1273  	dev_dbg(&board_dat->pdev->dev, "%s ENTRY\n", __func__);
e8b17b5b drivers/spi/spi_topcliff_pch.c Masayuki Ohtake 2010-10-08  1274  
e8b17b5b drivers/spi/spi_topcliff_pch.c Masayuki Ohtake 2010-10-08  1275  
e8b17b5b drivers/spi/spi_topcliff_pch.c Masayuki Ohtake 2010-10-08  1276  	/* reset PCH SPI h/w */
f016aeb6 drivers/spi/spi-topcliff-pch.c Tomoya MORINAGA 2011-06-07  1277  	pch_spi_reset(data->master);
e8b17b5b drivers/spi/spi_topcliff_pch.c Masayuki Ohtake 2010-10-08  1278  	dev_dbg(&board_dat->pdev->dev,
e8b17b5b drivers/spi/spi_topcliff_pch.c Masayuki Ohtake 2010-10-08  1279  		"%s pch_spi_reset invoked successfully\n", __func__);
e8b17b5b drivers/spi/spi_topcliff_pch.c Masayuki Ohtake 2010-10-08  1280  
65308c46 drivers/spi/spi_topcliff_pch.c Grant Likely    2010-09-29  1281  	dev_dbg(&board_dat->pdev->dev, "%s data->irq_reg_sts=true\n", __func__);
e8b17b5b drivers/spi/spi_topcliff_pch.c Masayuki Ohtake 2010-10-08  1282  
e8b17b5b drivers/spi/spi_topcliff_pch.c Masayuki Ohtake 2010-10-08 @1283  err_return:
e8b17b5b drivers/spi/spi_topcliff_pch.c Masayuki Ohtake 2010-10-08  1284  	if (retval != 0) {
e8b17b5b drivers/spi/spi_topcliff_pch.c Masayuki Ohtake 2010-10-08  1285  		dev_err(&board_dat->pdev->dev,
e8b17b5b drivers/spi/spi_topcliff_pch.c Masayuki Ohtake 2010-10-08  1286  			"%s FAIL:invoking pch_spi_free_resources\n", __func__);
f016aeb6 drivers/spi/spi-topcliff-pch.c Tomoya MORINAGA 2011-06-07  1287  		pch_spi_free_resources(board_dat, data);
e8b17b5b drivers/spi/spi_topcliff_pch.c Masayuki Ohtake 2010-10-08  1288  	}
e8b17b5b drivers/spi/spi_topcliff_pch.c Masayuki Ohtake 2010-10-08  1289  
e8b17b5b drivers/spi/spi_topcliff_pch.c Masayuki Ohtake 2010-10-08  1290  	dev_dbg(&board_dat->pdev->dev, "%s Return=%d\n", __func__, retval);
e8b17b5b drivers/spi/spi_topcliff_pch.c Masayuki Ohtake 2010-10-08  1291  

:::::: The code at line 1283 was first introduced by commit
:::::: e8b17b5b3f30252b5470dbbf54bc251ddc7fac17 spi/topcliff: Add topcliff platform controller hub (PCH) spi bus driver

:::::: TO: Masayuki Ohtake <masa-korg@dsn.okisemi.com>
:::::: CC: Grant Likely <grant.likely@secretlab.ca>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox

Patch

diff --git a/drivers/spi/spi-topcliff-pch.c b/drivers/spi/spi-topcliff-pch.c
index 93dfcee..621ec19 100644
--- a/drivers/spi/spi-topcliff-pch.c
+++ b/drivers/spi/spi-topcliff-pch.c
@@ -133,8 +133,6 @@  struct pch_spi_dma_ctrl {
  * @io_remap_addr:		The remapped PCI base address
  * @master:			Pointer to the SPI master structure
  * @work:			Reference to work queue handler
- * @wk:				Workqueue for carrying out execution of the
- *				requests
  * @wait:			Wait queue for waking up upon receiving an
  *				interrupt.
  * @transfer_complete:		Status of SPI Transfer
@@ -169,7 +167,6 @@  struct pch_spi_data {
 	unsigned long io_base_addr;
 	struct spi_master *master;
 	struct work_struct work;
-	struct workqueue_struct *wk;
 	wait_queue_head_t wait;
 	u8 transfer_complete;
 	u8 bcurrent_msg_processing;
@@ -517,8 +514,7 @@  static int pch_spi_transfer(struct spi_device *pspi, struct spi_message *pmsg)

 	dev_dbg(&pspi->dev, "%s - Invoked list_add_tail\n", __func__);

-	/* schedule work queue to run */
-	queue_work(data->wk, &data->work);
+	schedule_work(&data->work);
 	dev_dbg(&pspi->dev, "%s - Invoked queue work\n", __func__);

 	retval = 0;
@@ -674,7 +670,7 @@  static void pch_spi_nomore_transfer(struct pch_spi_data *data)
 		 *more messages)
 		 */
 		dev_dbg(&data->master->dev, "%s:Invoke queue_work\n", __func__);
-		queue_work(data->wk, &data->work);
+		schedule_work(&data->work);
 	} else if (data->board_dat->suspend_sts ||
 		   data->status == STATUS_EXITING) {
 		dev_dbg(&data->master->dev,
@@ -1266,14 +1262,7 @@  static void pch_spi_free_resources(struct pch_spi_board_data *board_dat,
 {
 	dev_dbg(&board_dat->pdev->dev, "%s ENTRY\n", __func__);

-	/* free workqueue */
-	if (data->wk != NULL) {
-		destroy_workqueue(data->wk);
-		data->wk = NULL;
-		dev_dbg(&board_dat->pdev->dev,
-			"%s destroy_workqueue invoked successfully\n",
-			__func__);
-	}
+	flush_work(&data->work);
 }

 static int pch_spi_get_resources(struct pch_spi_board_data *board_dat,
@@ -1283,14 +1272,6 @@  static int pch_spi_get_resources(struct pch_spi_board_data *board_dat,

 	dev_dbg(&board_dat->pdev->dev, "%s ENTRY\n", __func__);

-	/* create workqueue */
-	data->wk = create_singlethread_workqueue(KBUILD_MODNAME);
-	if (!data->wk) {
-		dev_err(&board_dat->pdev->dev,
-			"%s create_singlet hread_workqueue failed\n", __func__);
-		retval = -EBUSY;
-		goto err_return;
-	}

 	/* reset PCH SPI h/w */
 	pch_spi_reset(data->master);