Message ID | 20160702085334.GA28440@Karyakshetra (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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.
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 --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);
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