[PATCH v2] ASoC: Remove deprecated create_singlethread_workqueue

[PATCH v2] ASoC: Remove deprecated create_singlethread_workqueue

[Bhaktipriya Shridhar]

The workqueue "dac33_wq" queues a single work item &dac33->work and hence
doesn't require ordering. Also, it is not being used on a memory reclaim
path. Hence, it has been converted to use system_wq.

The work item has been flushed in dac33_soc_remove to ensure that
there are no pending tasks while disconnecting the driver.

The workqueue "post_msg_wq" queues a single work item
&drv->ipc_post_msg_wq and hence doesn't require ordering. Also, it is
not being used on a memory reclaim path. Hence, it has been converted to
use system_wq.

The work item has been flushed in sst_context_cleanup to ensure that
there are no pending tasks while disconnecting the driver.

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.

Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>
---
 Changes in v2:
	- Fixed heading to indicate that changes were made in sound/soc

 sound/soc/codecs/tlv320dac33.c | 17 ++++-------------
 sound/soc/intel/atom/sst/sst.c | 14 +++++---------
 2 files changed, 9 insertions(+), 22 deletions(-)

diff --git a/sound/soc/codecs/tlv320dac33.c b/sound/soc/codecs/tlv320dac33.c
index f7a6ce7..6822ac1 100644
--- a/sound/soc/codecs/tlv320dac33.c
+++ b/sound/soc/codecs/tlv320dac33.c
@@ -90,7 +90,6 @@ static const char *dac33_supply_names[DAC33_NUM_SUPPLIES] = {

 struct tlv320dac33_priv {
 	struct mutex mutex;
-	struct workqueue_struct *dac33_wq;
 	struct work_struct work;
 	struct snd_soc_codec *codec;
 	struct regulator_bulk_data supplies[DAC33_NUM_SUPPLIES];
@@ -771,7 +770,7 @@ static irqreturn_t dac33_interrupt_handler(int irq, void *dev)

 	/* Do not schedule the workqueue in Mode7 */
 	if (dac33->fifo_mode != DAC33_FIFO_MODE7)
-		queue_work(dac33->dac33_wq, &dac33->work);
+		schedule_work(&dac33->work);

 	return IRQ_HANDLED;
 }
@@ -1127,7 +1126,7 @@ static int dac33_pcm_trigger(struct snd_pcm_substream *substream, int cmd,
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
 		if (dac33->fifo_mode) {
 			dac33->state = DAC33_PREFILL;
-			queue_work(dac33->dac33_wq, &dac33->work);
+			schedule_work(&dac33->work);
 		}
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
@@ -1135,7 +1134,7 @@ static int dac33_pcm_trigger(struct snd_pcm_substream *substream, int cmd,
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
 		if (dac33->fifo_mode) {
 			dac33->state = DAC33_FLUSH;
-			queue_work(dac33->dac33_wq, &dac33->work);
+			schedule_work(&dac33->work);
 		}
 		break;
 	default:
@@ -1410,14 +1409,6 @@ static int dac33_soc_probe(struct snd_soc_codec *codec)
 			dac33->irq = -1;
 		}
 		if (dac33->irq != -1) {
-			/* Setup work queue */
-			dac33->dac33_wq =
-				create_singlethread_workqueue("tlv320dac33");
-			if (dac33->dac33_wq == NULL) {
-				free_irq(dac33->irq, codec);
-				return -ENOMEM;
-			}
-
 			INIT_WORK(&dac33->work, dac33_work);
 		}
 	}
@@ -1437,7 +1428,7 @@ static int dac33_soc_remove(struct snd_soc_codec *codec)

 	if (dac33->irq >= 0) {
 		free_irq(dac33->irq, dac33->codec);
-		destroy_workqueue(dac33->dac33_wq);
+		flush_work(&dac33->work);
 	}
 	return 0;
 }
diff --git a/sound/soc/intel/atom/sst/sst.c b/sound/soc/intel/atom/sst/sst.c
index a4b458e..8cfd0cf 100644
--- a/sound/soc/intel/atom/sst/sst.c
+++ b/sound/soc/intel/atom/sst/sst.c
@@ -76,7 +76,7 @@ static irqreturn_t intel_sst_interrupt_mrfld(int irq, void *context)
 		spin_unlock(&drv->ipc_spin_lock);

 		/* we can send more messages to DSP so trigger work */
-		queue_work(drv->post_msg_wq, &drv->ipc_post_msg_wq);
+		schedule_work(&drv->ipc_post_msg_wq);
 		retval = IRQ_HANDLED;
 	}

@@ -212,10 +212,6 @@ static int sst_workqueue_init(struct intel_sst_drv *ctx)
 	INIT_WORK(&ctx->ipc_post_msg_wq, sst_process_pending_msg);
 	init_waitqueue_head(&ctx->wait_queue);

-	ctx->post_msg_wq =
-		create_singlethread_workqueue("sst_post_msg_wq");
-	if (!ctx->post_msg_wq)
-		return -EBUSY;
 	return 0;
 }

@@ -318,7 +314,6 @@ int sst_context_init(struct intel_sst_drv *ctx)
 	return 0;

 do_free_mem:
-	destroy_workqueue(ctx->post_msg_wq);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(sst_context_init);
@@ -330,7 +325,7 @@ void sst_context_cleanup(struct intel_sst_drv *ctx)
 	sst_unregister(ctx->dev);
 	sst_set_fw_state_locked(ctx, SST_SHUTDOWN);
 	flush_scheduled_work();
-	destroy_workqueue(ctx->post_msg_wq);
+	flush_work(&ctx->ipc_post_msg_wq);
 	pm_qos_remove_request(ctx->qos);
 	kfree(ctx->fw_sg_list.src);
 	kfree(ctx->fw_sg_list.dst);
@@ -414,7 +409,7 @@ static int intel_sst_runtime_suspend(struct device *dev)
 	sst_set_fw_state_locked(ctx, SST_RESET);

 	synchronize_irq(ctx->irq_num);
-	flush_workqueue(ctx->post_msg_wq);
+	flush_work(&ctx->ipc_post_msg_wq);

 	ctx->ops->reset(ctx);
 	/* save the shim registers because PMC doesn't save state */
@@ -445,8 +440,9 @@ static int intel_sst_suspend(struct device *dev)
 			return -EBUSY;
 		}
 	}
+
 	synchronize_irq(ctx->irq_num);
-	flush_workqueue(ctx->post_msg_wq);
+	flush_work(ctx->ipc_post_msg_wq);

 	/* Move the SST state to Reset */
 	sst_set_fw_state_locked(ctx, SST_RESET);
--
2.1.4

Re: [PATCH v2] ASoC: Remove deprecated create_singlethread_workqueue

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2289 bytes --]

Hi Bhaktipriya,

[auto build test ERROR on asoc/for-next]
[also build test ERROR on v4.8-rc4 next-20160825]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]

url:    https://github.com/0day-ci/linux/commits/Bhaktipriya-Shridhar/ASoC-tlv320dac33-Remove-deprecated-create_singlethread_workqueue/20160831-023349
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   sound/soc/intel/atom/sst/sst.c: In function 'intel_sst_suspend':
>> sound/soc/intel/atom/sst/sst.c:446:13: error: incompatible type for argument 1 of 'flush_work'
     flush_work(ctx->ipc_post_msg_wq);
                ^~~
   In file included from include/linux/srcu.h:34:0,
                    from include/linux/notifier.h:15,
                    from include/linux/memory_hotplug.h:6,
                    from include/linux/mmzone.h:777,
                    from include/linux/gfp.h:5,
                    from include/linux/kmod.h:22,
                    from include/linux/module.h:13,
                    from sound/soc/intel/atom/sst/sst.c:22:
   include/linux/workqueue.h:444:13: note: expected 'struct work_struct *' but argument is of type 'struct work_struct'
    extern bool flush_work(struct work_struct *work);
                ^~~~~~~~~~

vim +/flush_work +446 sound/soc/intel/atom/sst/sst.c

   440				dev_err(dev, "stream %d is running, cant susupend, abort\n", i);
   441				return -EBUSY;
   442			}
   443		}
   444	
   445		synchronize_irq(ctx->irq_num);
 > 446		flush_work(ctx->ipc_post_msg_wq);
   447	
   448		/* Move the SST state to Reset */
   449		sst_set_fw_state_locked(ctx, SST_RESET);

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

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 55564 bytes --]