login
Search
Hello, This patchset examines each user of create_workqueue() and update them to better fit the new workqueue implementation. For backward compatibility, create_workqueue() is mapped to alloc_workqueue(@name, WQ_MEM_RECLAIM, 1) but for a lot of its users, WQ_MEM_RECLAIM is not required, @max_active of 1 is unnecessarily strict, or the dedicated workqueue simply isn't needed. * As the system workqueues can now host higher concurrency, a lot of users can drop dedicated workqueues and use the system ones. * There are cases where keeping dedicated workqueues are necessary. e.g. They are depended upon during memory reclaim, need to be highpri or CPU intensive, serve work items which free themselves (and thus work items can't be explicitly queued), or need to limit max concurrency level from workqueue. In these cases, alloc_workqueue() calls with better fitting parameters are used. * This patchset mostly concentrates on converting create_workqueue() users but it also updates general workqueue usages when it makes sense to update them together. Please read description of each patch for deatils. This simplifies code, removes unnecessary rescuer kernel thread and makes workqueue utilized more efficiently. This patchset contains the following 32 patches. 0001-arm-omap-use-system_wq-in-mailbox.patch 0002-powerpc-cell-use-system_wq-in-cpufreq_spudemand.patch 0003-block-make-kblockd_workqueue-smarter.patch 0004-bio-integrity-mark-kintegrityd_wq-highpri-and-CPU-in.patch 0005-crypto-mark-crypto-workqueues-CPU_INTENSIVE.patch 0006-acpi-kacpi-_wq-don-t-need-WQ_MEM_RECLAIM.patch 0007-cpufreq-use-system_wq-instead-of-dedicated-workqueue.patch 0008-drm-nouveau-use-system_wq-instead-of-dev_priv-wq.patch 0009-drm-radeon-use-system_wq-instead-of-dev_priv-wq.patch 0010-input-tps6507x-ts-use-system_wq-instead-of-dedicated.patch 0011-v4l-cx18-update-workqueue-usage.patch 0012-i2o-use-alloc_workqueue-instead-of-create_workqueue.patch ...
With cmwq, there's no reason to use a separate workqueue for mailbox. Use the system_wq instead. mbox->rxq->work is sync flushed in omap_mbox_fini() to make sure it's not running on any cpu, which makes sure that no mbox work is running when omap_mbox_exit() is entered. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Tony Lindgren <tony@atomide.com> Cc: linux-omap@vger.kernel.org --- Only compile tested. Please feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. arch/arm/plat-omap/mailbox.c | 10 ++-------- 1 files changed, 2 insertions(+), 8 deletions(-) diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c index d2fafb8..5bc4d7b 100644 --- a/arch/arm/plat-omap/mailbox.c +++ b/arch/arm/plat-omap/mailbox.c @@ -31,7 +31,6 @@ #include <plat/mailbox.h> -static struct workqueue_struct *mboxd; static struct omap_mbox **mboxes; static bool rq_full; @@ -186,7 +185,7 @@ static void __mbox_rx_interrupt(struct omap_mbox *mbox) /* no more messages in the fifo. clear IRQ source. */ ack_mbox_irq(mbox, IRQ_RX); nomem: - queue_work(mboxd, &mbox->rxq->work); + schedule_work(&mbox->rxq->work); } static irqreturn_t mbox_interrupt(int irq, void *p) @@ -291,7 +290,7 @@ static void omap_mbox_fini(struct omap_mbox *mbox) { free_irq(mbox->irq, mbox); tasklet_kill(&mbox->txq->tasklet); - flush_work(&mbox->rxq->work); + flush_work_sync(&mbox->rxq->work); mbox_queue_free(mbox->txq); mbox_queue_free(mbox->rxq); @@ -385,10 +384,6 @@ static int __init omap_mbox_init(void) if (err) return err; - mboxd = create_workqueue("mboxd"); - if (!mboxd) - return -ENOMEM; - /* kfifo size sanity check: alignment and minimal size */ mbox_kfifo_size = ALIGN(mbox_kfifo_size, sizeof(mbox_msg_t)); mbox_kfifo_size = max_t(unsigned int, mbox_kfifo_size, sizeof(mbox_msg_t)); @@ -399,7 +394,6 @@ subsys_initcall(omap_mbox_init); static void __exit omap_mbox_exit(void) ...
Tejun, This was changed to dedicated work queue because of performance issues when there is heavy mailbox traffic between the cores. Reference: -- Thank you, Best regards, Hari Kanigeri --
Hello, Using dedicated workqueue or system_wq doesn't make any difference in terms of execution latency anymore. Sleeping work items no longer delay execution of other work items. If mailbox is very latency sensitive, it might make sense to create a HIGHPRI workqueue but that usually isn't necessary. Thank you. -- tejun --
Work items processed by kintegrityd_wq won't block much, may burn a
lot of CPU cycles and affect IO latency. Use alloc_workqueue() to
mark it highpri and CPU intensive with max concurrency of 1.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
---
Only compile tested. Please feel free to take it into the subsystem
tree or simply ack - I'll route it through the wq tree.
Thanks.
fs/bio-integrity.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/fs/bio-integrity.c b/fs/bio-integrity.c
index 4d0ff5e..e49cce2 100644
--- a/fs/bio-integrity.c
+++ b/fs/bio-integrity.c
@@ -782,7 +782,12 @@ void __init bio_integrity_init(void)
{
unsigned int i;
- kintegrityd_wq = create_workqueue("kintegrityd");
+ /*
+ * kintegrityd won't block much but may burn a lot of CPU cycles.
+ * Make it highpri CPU intensive wq with max concurrency of 1.
+ */
+ kintegrityd_wq = alloc_workqueue("kintegrityd", WQ_MEM_RECLAIM |
+ WQ_HIGHPRI | WQ_CPU_INTENSIVE, 1);
if (!kintegrityd_wq)
panic("Failed to create kintegrityd\n");
--
1.7.1
--
ACPI workqueues aren't used during memory reclaming. Use alloc_workqueue() to create workqueues w/o rescuers. If the purpose of the separation between kacpid_wq and kacpi_notify_wq was to give notifications better response time, kacpi_notify_wq can be dropped and kacpi_wq can be created with higher @max_active. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Len Brown <lenb@kernel.org> Cc: linux-acpi@vger.kernel.org --- Only compile tested. Please feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. drivers/acpi/osl.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c index 055d7b7..c961e18 100644 --- a/drivers/acpi/osl.c +++ b/drivers/acpi/osl.c @@ -1573,9 +1573,9 @@ acpi_status __init acpi_os_initialize(void) acpi_status __init acpi_os_initialize1(void) { - kacpid_wq = create_workqueue("kacpid"); - kacpi_notify_wq = create_workqueue("kacpi_notify"); - kacpi_hotplug_wq = create_workqueue("kacpi_hotplug"); + kacpid_wq = alloc_workqueue("kacpid", 0, 1); + kacpi_notify_wq = alloc_workqueue("kacpi_notify", 0, 1); + kacpi_hotplug_wq = alloc_workqueue("kacpi_hotplug", 0, 1); BUG_ON(!kacpid_wq); BUG_ON(!kacpi_notify_wq); BUG_ON(!kacpi_hotplug_wq); -- 1.7.1 --
With cmwq, there's no reason for nouveau to use a dedicated workqueue. Drop dev_priv->wq and use system_wq instead. Because nouveau_irq_uninstall() may be called from unsleepable context, the work items can't be flushed from there. Instead, init and flush from nouveau_load/unload(). Signed-off-by: Tejun Heo <tj@kernel.org> Cc: David Airlie <airlied@linux.ie> Cc: dri-devel@lists.freedesktop.org --- Only compile tested. Please feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. drivers/gpu/drm/nouveau/nouveau_drv.h | 1 - drivers/gpu/drm/nouveau/nouveau_irq.c | 9 --------- drivers/gpu/drm/nouveau/nouveau_state.c | 19 ++++++++++--------- drivers/gpu/drm/nouveau/nv50_display.c | 4 ++-- 4 files changed, 12 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h b/drivers/gpu/drm/nouveau/nouveau_drv.h index 1c7db64..2ecf875 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drv.h +++ b/drivers/gpu/drm/nouveau/nouveau_drv.h @@ -580,7 +580,6 @@ struct drm_nouveau_private { struct nouveau_bo *vga_ram; - struct workqueue_struct *wq; struct work_struct irq_work; struct work_struct hpd_work; diff --git a/drivers/gpu/drm/nouveau/nouveau_irq.c b/drivers/gpu/drm/nouveau/nouveau_irq.c index 7bfd9e6..7d05a06 100644 --- a/drivers/gpu/drm/nouveau/nouveau_irq.c +++ b/drivers/gpu/drm/nouveau/nouveau_irq.c @@ -52,17 +52,8 @@ static int nouveau_ratelimit(void) void nouveau_irq_preinstall(struct drm_device *dev) { - struct drm_nouveau_private *dev_priv = dev->dev_private; - /* Master disable */ nv_wr32(dev, NV03_PMC_INTR_EN_0, 0); - - if (dev_priv->card_type >= NV_50) { - INIT_WORK(&dev_priv->irq_work, nv50_display_irq_handler_bh); - INIT_WORK(&dev_priv->hpd_work, nv50_display_irq_hotplug_bh); - spin_lock_init(&dev_priv->hpd_state.lock); - INIT_LIST_HEAD(&dev_priv->vbl_waiting); - } } int diff --git a/drivers/gpu/drm/nouveau/nouveau_state.c ...
With cmwq, there's no reason to use separate workqueues in
iwmc3200top. Drop them and use system_wq instead. The used work
items are sync flushed before driver detach.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Tomas Winkler <tomas.winkler@intel.com>
---
Only compile tested. Please feel free to take it into the subsystem
tree or simply ack - I'll route it through the wq tree.
Thanks.
drivers/misc/iwmc3200top/iwmc3200top.h | 4 +---
drivers/misc/iwmc3200top/main.c | 14 +++++---------
2 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/misc/iwmc3200top/iwmc3200top.h b/drivers/misc/iwmc3200top/iwmc3200top.h
index 740ff07..620973e 100644
--- a/drivers/misc/iwmc3200top/iwmc3200top.h
+++ b/drivers/misc/iwmc3200top/iwmc3200top.h
@@ -183,9 +183,7 @@ struct iwmct_priv {
u32 barker;
struct iwmct_dbg dbg;
- /* drivers work queue */
- struct workqueue_struct *wq;
- struct workqueue_struct *bus_rescan_wq;
+ /* drivers work items */
struct work_struct bus_rescan_worker;
struct work_struct isr_worker;
diff --git a/drivers/misc/iwmc3200top/main.c b/drivers/misc/iwmc3200top/main.c
index c73cef2..727af07 100644
--- a/drivers/misc/iwmc3200top/main.c
+++ b/drivers/misc/iwmc3200top/main.c
@@ -89,7 +89,7 @@ static void op_top_message(struct iwmct_priv *priv, struct top_msg *msg)
switch (msg->hdr.opcode) {
case OP_OPR_ALIVE:
LOG_INFO(priv, FW_MSG, "Got ALIVE from device, wake rescan\n");
- queue_work(priv->bus_rescan_wq, &priv->bus_rescan_worker);
+ schedule_work(&priv->bus_rescan_worker);
break;
default:
LOG_INFO(priv, FW_MSG, "Received msg opcode 0x%X\n",
@@ -360,7 +360,7 @@ static void iwmct_irq(struct sdio_func *func)
/* clear the function's interrupt request bit (write 1 to clear) */
sdio_writeb(func, 1, IWMC_SDIO_INTR_CLEAR_ADDR, &ret);
- queue_work(priv->wq, &priv->isr_worker);
+ schedule_work(&priv->isr_worker);
LOG_TRACE(priv, IRQ, "exit iwmct_irq\n");
@@ -506,10 +506,6 @@ static int ...The target driver is not in the memory reclaim path and doesn't need a dedicated workqueue. Drop vtgtd and use system_wq instead. The used work item is sync flushed on removal. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com> Cc: linux-scsi@vger.kernel.org --- Only compile tested. Please feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. drivers/scsi/ibmvscsi/ibmvstgt.c | 15 ++++----------- 1 files changed, 4 insertions(+), 11 deletions(-) diff --git a/drivers/scsi/ibmvscsi/ibmvstgt.c b/drivers/scsi/ibmvscsi/ibmvstgt.c index 2256bab..47fc632 100644 --- a/drivers/scsi/ibmvscsi/ibmvstgt.c +++ b/drivers/scsi/ibmvscsi/ibmvstgt.c @@ -74,7 +74,6 @@ struct vio_port { struct srp_rport *rport; }; -static struct workqueue_struct *vtgtd; static struct scsi_transport_template *ibmvstgt_transport_template; /* @@ -546,7 +545,7 @@ static irqreturn_t ibmvstgt_interrupt(int dummy, void *data) struct vio_port *vport = target_to_port(target); vio_disable_interrupts(vport->dma_dev); - queue_work(vtgtd, &vport->crq_work); + schedule_work(&vport->crq_work); return IRQ_HANDLED; } @@ -900,6 +899,7 @@ static int ibmvstgt_remove(struct vio_dev *dev) crq_queue_destroy(target); srp_remove_host(shost); scsi_remove_host(shost); + flush_work_sync(&vport->crq_work); scsi_tgt_free_queue(shost); srp_target_free(target); kfree(vport); @@ -967,21 +967,15 @@ static int __init ibmvstgt_init(void) if (!ibmvstgt_transport_template) return err; - vtgtd = create_workqueue("ibmvtgtd"); - if (!vtgtd) - goto release_transport; - err = get_system_info(); if (err) - goto destroy_wq; + goto release_transport; err = vio_register_driver(&ibmvstgt_driver); if (err) - goto destroy_wq; + goto release_transport; return 0; -destroy_wq: - destroy_workqueue(vtgtd); ...
[ ... ] Hi Tejun, No matter which storage target will go upstream, I'm afraid that this patch will conflict with it. See e.g. http://dchs.spinics.net/lists/linux-scsi/msg49105.html. Bart. --
Hello, Ah, okay. The full conversion is gonna span several devel cycles anyway. I'll re-convert ibmvstgt when the dust settles down. Thank you. -- tejun --
With cmwq, there's no reason to use separate workqueues in libertas[_tf] drivers. Drop them and use system_wq instead. All used work items are sync flushed/canceled on driver detach. Cc: Dan Williams <dcbw@redhat.com> Cc: libertas-dev@lists.infradead.org --- Only compile tested. Please feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. drivers/net/wireless/libertas/if_sdio.c | 10 +++------- drivers/net/wireless/libertas_tf/cmd.c | 6 +++--- drivers/net/wireless/libertas_tf/libertas_tf.h | 2 -- drivers/net/wireless/libertas_tf/main.c | 16 ++++------------ 4 files changed, 10 insertions(+), 24 deletions(-) diff --git a/drivers/net/wireless/libertas/if_sdio.c b/drivers/net/wireless/libertas/if_sdio.c index b4de0ca..257f664 100644 --- a/drivers/net/wireless/libertas/if_sdio.c +++ b/drivers/net/wireless/libertas/if_sdio.c @@ -129,7 +129,6 @@ struct if_sdio_card { spinlock_t lock; struct if_sdio_packet *packets; - struct workqueue_struct *workqueue; struct work_struct packet_worker; u8 rx_unit; @@ -828,7 +827,7 @@ static int if_sdio_host_to_card(struct lbs_private *priv, spin_unlock_irqrestore(&card->lock, flags); - queue_work(card->workqueue, &card->packet_worker); + schedule_work(&card->packet_worker); ret = 0; @@ -986,7 +985,6 @@ static int if_sdio_probe(struct sdio_func *func, } spin_lock_init(&card->lock); - card->workqueue = create_workqueue("libertas_sdio"); INIT_WORK(&card->packet_worker, if_sdio_host_to_card_worker); /* Check if we support this card */ @@ -1114,7 +1112,7 @@ out: return ret; err_activate_card: - flush_workqueue(card->workqueue); + flush_work_sync(&card->packet_worker); lbs_remove_card(priv); reclaim: sdio_claim_host(func); @@ -1125,7 +1123,6 @@ disable: release: sdio_release_host(func); free: - destroy_workqueue(card->workqueue); while (card->packets) { packet = card->packets; ...
dlm isn't depended upon during memory reclaim and so are its two workqueues. Convert to alloc[_ordered]_workqueue() without WQ_MEM_RECLAIM. While at it, fix workqueue allocation failure handling. They return %NULL on failure not ERR_PTR() value. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Christine Caulfield <ccaulfie@redhat.com> Cc: David Teigland <teigland@redhat.com> Cc: cluster-devel@redhat.com --- Only compile tested. Please feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. fs/dlm/lowcomms.c | 19 ++++++++----------- 1 files changed, 8 insertions(+), 11 deletions(-) diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c index 37a34c2..9aad7c0 100644 --- a/fs/dlm/lowcomms.c +++ b/fs/dlm/lowcomms.c @@ -1430,20 +1430,17 @@ static void work_stop(void) static int work_start(void) { - int error; - recv_workqueue = create_workqueue("dlm_recv"); - error = IS_ERR(recv_workqueue); - if (error) { - log_print("can't start dlm_recv %d", error); - return error; + recv_workqueue = alloc_workqueue("dlm_recv", 0, 1); + if (!recv_workqueue) { + log_print("can't start dlm_recv"); + return -ENOMEM; } - send_workqueue = create_singlethread_workqueue("dlm_send"); - error = IS_ERR(send_workqueue); - if (error) { - log_print("can't start dlm_send %d", error); + send_workqueue = alloc_ordered_workqueue("dlm_send", 0); + if (!send_workqueue) { + log_print("can't start dlm_send"); destroy_workqueue(recv_workqueue); - return error; + return -ENOMEM; } return 0; -- 1.7.1 --
Hi, You already acked a patch to do this, which is in the DLM git tree, waiting for the next merge window: http://git.kernel.org/?p=linux/kernel/git/teigland/dlm.git;a=commitdiff;h=dcce240ead80... Steve. --
Ooh, I see. Hmmm... maybe I've asked this before but do you really need WQ_MEM_RECLAIM there? Is dlm used during memory reclaim? Thanks. -- tejun --
Hi, Yes, it is in case of deallocation of inodes, so we have to be careful about that, Steve. --
Hello again, I see. I'm still slightly unsure about the WQ_MEM_RECLAIM rules when it comes to distributed file systems and stuff related to them, but I think it basically comes down to whether the code can be in the swap path or not. e.g. for nfs backed write shared mapped files, nfs definitely can be used during memory reclaim _but_ it's not guaranteed to make forward progress under memory pressure and thus can't be depended upon and shouldn't be used for swap. IOW, it may free memory but it's not in the critical path which should guarantee forward progress. Isn't dlm the same case? Thank you. -- tejun --
Hi, No, the issue arises in case an inode is being ejected from cache with a 0 link count (and this need deallocation). At this point the dlm is required in order to coordinate which node will perform the deallocation, but if it blocks on memory reclaim (via the fs, so GFP_NOFS is ok, but GFP_KERNEL is not) then it can land up calling back into the fs via a shrink of the dcache which can potentially then also call back into the dlm. Since there is no workqueue option to indicate that GFP_NOFS is ok, but GFP_KERNEL is not, I think in the workqueue case we must mark it as WQ_MEM_RECLAIM. So far as I can tell thats a limitation of task creation which was the reason for the pre-allocated rescuer thread. I'd rather be safe than sorry in this case, Steve. --
Hello, Yeap, in such case, WQ_MEM_RECLAIM seems like the right choice. It may call into GFP_KERNEL path again, but the dependency chain isn't hard. It will be broken pretty soon and rescuer will kick in. As the case is rather special, it might be a good idea to explain why WQ_MEM_RECLAIM is necessary for future references? Thank you. -- tejun --
Now that cmwq can handle high concurrency, it's more efficient to use work than a dedicated kthread. Convert p9_poll_proc() to a work function for p9_poll_work and make p9_pollwake() schedule it on each poll event. The work is sync flushed on module exit. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Eric Van Hensbergen <ericvh@gmail.com> Cc: Ron Minnich <rminnich@sandia.gov> Cc: Latchesar Ionkov <lucho@ionkov.net> Cc: v9fs-developer@lists.sourceforge.net --- Lightly tested with userland server. Seems to work fine. Please feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. net/9p/trans_fd.c | 32 ++++++++------------------------ 1 files changed, 8 insertions(+), 24 deletions(-) diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c index e9f797d..a30471e 100644 --- a/net/9p/trans_fd.c +++ b/net/9p/trans_fd.c @@ -153,9 +153,11 @@ struct p9_conn { unsigned long wsched; }; +static void p9_poll_workfn(struct work_struct *work); + static DEFINE_SPINLOCK(p9_poll_lock); static LIST_HEAD(p9_poll_pending_list); -static struct task_struct *p9_poll_task; +static DECLARE_WORK(p9_poll_work, p9_poll_workfn); static void p9_mux_poll_stop(struct p9_conn *m) { @@ -515,15 +517,14 @@ static int p9_pollwake(wait_queue_t *wait, unsigned mode, int sync, void *key) container_of(wait, struct p9_poll_wait, wait); struct p9_conn *m = pwait->conn; unsigned long flags; - DECLARE_WAITQUEUE(dummy_wait, p9_poll_task); spin_lock_irqsave(&p9_poll_lock, flags); if (list_empty(&m->poll_pending_link)) list_add_tail(&m->poll_pending_link, &p9_poll_pending_list); spin_unlock_irqrestore(&p9_poll_lock, flags); - /* perform the default wake up operation */ - return default_wake_function(&dummy_wait, mode, sync, key); + schedule_work(&p9_poll_work); + return 1; } /** @@ -1046,12 +1047,12 @@ static struct p9_trans_module p9_fd_trans = { * */ -static int p9_poll_proc(void *a) +static void ...
rxrpc_workqueue isn't depended upon while reclaiming memory. Convert to alloc_workqueue() without WQ_MEM_RECLAIM. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: David Howells <dhowells@redhat.com> Cc: linux-afs@lists.infradead.org --- Only compile tested. Please feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. net/rxrpc/af_rxrpc.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/net/rxrpc/af_rxrpc.c b/net/rxrpc/af_rxrpc.c index 0b9bb20..74c064c 100644 --- a/net/rxrpc/af_rxrpc.c +++ b/net/rxrpc/af_rxrpc.c @@ -808,7 +808,7 @@ static int __init af_rxrpc_init(void) goto error_call_jar; } - rxrpc_workqueue = create_workqueue("krxrpcd"); + rxrpc_workqueue = alloc_workqueue("krxrpcd", 0, 1); if (!rxrpc_workqueue) { printk(KERN_NOTICE "RxRPC: Failed to allocate work queue\n"); goto error_work_queue; -- 1.7.1 --
ocfs2_quota_wq is not depended upon during memory reclaim and, with cmwq, there's no reason to use a dedicated workqueue. Drop ocfs2_quota_wq and use system_wq instead. dqi_sync_work is already sync canceled on quota disable and no further synchronization is necessary. This change makes ocfs2_quota_setup/shutdown() noops. Both functions removed. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Mark Fasheh <mfasheh@suse.com> Cc: Joel Becker <joel.becker@oracle.com> --- Only compile tested. Please feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. fs/ocfs2/quota.h | 3 --- fs/ocfs2/quota_global.c | 27 ++++----------------------- fs/ocfs2/super.c | 7 ------- 3 files changed, 4 insertions(+), 33 deletions(-) diff --git a/fs/ocfs2/quota.h b/fs/ocfs2/quota.h index 196fcb5..d5ab56c 100644 --- a/fs/ocfs2/quota.h +++ b/fs/ocfs2/quota.h @@ -114,7 +114,4 @@ int ocfs2_local_write_dquot(struct dquot *dquot); extern const struct dquot_operations ocfs2_quota_operations; extern struct quota_format_type ocfs2_quota_format; -int ocfs2_quota_setup(void); -void ocfs2_quota_shutdown(void); - #endif /* _OCFS2_QUOTA_H */ diff --git a/fs/ocfs2/quota_global.c b/fs/ocfs2/quota_global.c index 4607923..a73f641 100644 --- a/fs/ocfs2/quota_global.c +++ b/fs/ocfs2/quota_global.c @@ -63,8 +63,6 @@ * write to gf */ -static struct workqueue_struct *ocfs2_quota_wq = NULL; - static void qsync_work_fn(struct work_struct *work); static void ocfs2_global_disk2memdqb(struct dquot *dquot, void *dp) @@ -400,8 +398,8 @@ int ocfs2_global_read_info(struct super_block *sb, int type) OCFS2_QBLK_RESERVED_SPACE; oinfo->dqi_gi.dqi_qtree_depth = qtree_depth(&oinfo->dqi_gi); INIT_DELAYED_WORK(&oinfo->dqi_sync_work, qsync_work_fn); - queue_delayed_work(ocfs2_quota_wq, &oinfo->dqi_sync_work, - msecs_to_jiffies(oinfo->dqi_syncms)); + schedule_delayed_work(&oinfo->dqi_sync_work, + ...
Convert create_workqueue() to alloc_workqueue(). This is an identity conversion. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: "Theodore Ts'o" <tytso@mit.edu> Cc: Andreas Dilger <adilger.kernel@dilger.ca> Cc: linux-ext4@vger.kernel.org --- It might be helpful to use higher max concurrency. I don't think it would make a lot of difference tho. Please feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. fs/ext4/super.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-) diff --git a/fs/ext4/super.c b/fs/ext4/super.c index fb15c9c..f682c40 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -3495,7 +3495,12 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent) percpu_counter_set(&sbi->s_dirtyblocks_counter, 0); no_journal: - EXT4_SB(sb)->dio_unwritten_wq = create_workqueue("ext4-dio-unwritten"); + /* + * The maximum number of concurrent works can be high and + * concurrency isn't really necessary. Limit it to 1. + */ + EXT4_SB(sb)->dio_unwritten_wq = + alloc_workqueue("ext4-dio-unwritten", WQ_MEM_RECLAIM, 1); if (!EXT4_SB(sb)->dio_unwritten_wq) { printk(KERN_ERR "EXT4-fs: failed to create DIO workqueue\n"); goto failed_mount_wq; -- 1.7.1 --
With cmwq, there's no reason to use dedicated rds_ib_fmr_wq - it's not
in the memory reclaim path and the maximum number of concurrent work
items is bound by the number of devices. Drop it and use system_wq
instead. This rds_ib_fmr_init/exit() noops. Both removed.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Andy Grover <andy.grover@oracle.com>
---
Only compile tested. Please feel free to take it into the subsystem
tree or simply ack - I'll route it through the wq tree.
Thanks.
net/rds/ib.c | 9 +--------
net/rds/ib.h | 2 --
net/rds/ib_rdma.c | 27 +++------------------------
3 files changed, 4 insertions(+), 34 deletions(-)
diff --git a/net/rds/ib.c b/net/rds/ib.c
index 4123967..cce19f9 100644
--- a/net/rds/ib.c
+++ b/net/rds/ib.c
@@ -364,7 +364,6 @@ void rds_ib_exit(void)
rds_ib_sysctl_exit();
rds_ib_recv_exit();
rds_trans_unregister(&rds_ib_transport);
- rds_ib_fmr_exit();
}
struct rds_transport rds_ib_transport = {
@@ -400,13 +399,9 @@ int rds_ib_init(void)
INIT_LIST_HEAD(&rds_ib_devices);
- ret = rds_ib_fmr_init();
- if (ret)
- goto out;
-
ret = ib_register_client(&rds_ib_client);
if (ret)
- goto out_fmr_exit;
+ goto out;
ret = rds_ib_sysctl_init();
if (ret)
@@ -430,8 +425,6 @@ out_sysctl:
rds_ib_sysctl_exit();
out_ibreg:
rds_ib_unregister_client();
-out_fmr_exit:
- rds_ib_fmr_exit();
out:
return ret;
}
diff --git a/net/rds/ib.h b/net/rds/ib.h
index e34ad03..4297d92 100644
--- a/net/rds/ib.h
+++ b/net/rds/ib.h
@@ -307,8 +307,6 @@ void *rds_ib_get_mr(struct scatterlist *sg, unsigned long nents,
void rds_ib_sync_mr(void *trans_private, int dir);
void rds_ib_free_mr(void *trans_private, int invalidate);
void rds_ib_flush_mrs(void);
-int rds_ib_fmr_init(void);
-void rds_ib_fmr_exit(void);
/* ib_recv.c */
int rds_ib_recv_init(void);
diff --git a/net/rds/ib_rdma.c b/net/rds/ib_rdma.c
index 18a833c..819c35a 100644
--- a/net/rds/ib_rdma.c
+++ b/net/rds/ib_rdma.c
@@ -38,8 ...With cmwq, there's no reason to use a dedicated workqueue in trans_fd. Drop p9_mux_wq and use system_wq instead. The used work items are already sync canceled in p9_conn_destroy() and doesn't require further synchronization. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Eric Van Hensbergen <ericvh@gmail.com> Cc: Ron Minnich <rminnich@sandia.gov> Cc: Latchesar Ionkov <lucho@ionkov.net> Cc: v9fs-developer@lists.sourceforge.net --- Lightly tested with userland server. Seems to work fine. Please feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. net/9p/trans_fd.c | 20 +++++--------------- 1 files changed, 5 insertions(+), 15 deletions(-) diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c index 078eb16..e9f797d 100644 --- a/net/9p/trans_fd.c +++ b/net/9p/trans_fd.c @@ -155,7 +155,6 @@ struct p9_conn { static DEFINE_SPINLOCK(p9_poll_lock); static LIST_HEAD(p9_poll_pending_list); -static struct workqueue_struct *p9_mux_wq; static struct task_struct *p9_poll_task; static void p9_mux_poll_stop(struct p9_conn *m) @@ -384,7 +383,7 @@ static void p9_read_work(struct work_struct *work) if (n & POLLIN) { P9_DPRINTK(P9_DEBUG_TRANS, "sched read work %p\n", m); - queue_work(p9_mux_wq, &m->rq); + schedule_work(&m->rq); } else clear_bit(Rworksched, &m->wsched); } else @@ -497,7 +496,7 @@ static void p9_write_work(struct work_struct *work) if (n & POLLOUT) { P9_DPRINTK(P9_DEBUG_TRANS, "sched write work %p\n", m); - queue_work(p9_mux_wq, &m->wq); + schedule_work(&m->wq); } else clear_bit(Wworksched, &m->wsched); } else @@ -629,7 +628,7 @@ static void p9_poll_mux(struct p9_conn *m) P9_DPRINTK(P9_DEBUG_TRANS, "mux %p can read\n", m); if (!test_and_set_bit(Rworksched, &m->wsched)) { P9_DPRINTK(P9_DEBUG_TRANS, "sched read work %p\n", m); - queue_work(p9_mux_wq, &m->rq); + schedule_work(&m->rq); } } @@ -639,7 +638,7 @@ static void ...
Convert from create[_singlethread]_workqueue() to alloc_workqueue(). * xfsdatad_workqueue and xfsconvertd_workqueue are identity converted. Using higher concurrency limit might be useful but given the complexity of workqueue usage in xfs, proceeding cautiously seems better. * xfs_mru_reap_wq is converted to non-ordered workqueue with max concurrency of 1 as the work items don't require any specific ordering and already have proper synchronization. It seems it was singlethreaded to save worker threads, which is no longer a concern. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Alex Elder <aelder@sgi.com> Cc: xfs-masters@oss.sgi.com Cc: Christoph Hellwig <hch@infradead.org> --- Only compile tested. Please feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. fs/xfs/linux-2.6/xfs_buf.c | 5 +++-- fs/xfs/xfs_mru_cache.c | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/fs/xfs/linux-2.6/xfs_buf.c b/fs/xfs/linux-2.6/xfs_buf.c index 4c5deb6..d6da590 100644 --- a/fs/xfs/linux-2.6/xfs_buf.c +++ b/fs/xfs/linux-2.6/xfs_buf.c @@ -1944,11 +1944,12 @@ xfs_buf_init(void) if (!xfslogd_workqueue) goto out_free_buf_zone; - xfsdatad_workqueue = create_workqueue("xfsdatad"); + xfsdatad_workqueue = alloc_workqueue("xfsdatad", WQ_MEM_RECLAIM, 1); if (!xfsdatad_workqueue) goto out_destroy_xfslogd_workqueue; - xfsconvertd_workqueue = create_workqueue("xfsconvertd"); + xfsconvertd_workqueue = alloc_workqueue("xfsconvertd", + WQ_MEM_RECLAIM, 1); if (!xfsconvertd_workqueue) goto out_destroy_xfsdatad_workqueue; diff --git a/fs/xfs/xfs_mru_cache.c b/fs/xfs/xfs_mru_cache.c index 45ce15d..2acdc77 100644 --- a/fs/xfs/xfs_mru_cache.c +++ b/fs/xfs/xfs_mru_cache.c @@ -309,7 +309,7 @@ xfs_mru_cache_init(void) if (!xfs_mru_elem_zone) goto out; - xfs_mru_reap_wq = create_singlethread_workqueue("xfs_mru_cache"); + xfs_mru_reap_wq = alloc_workqueue("xfs_mru_cache", ...
The maximum number of concurrent work items queued on commit_wq is bound by the number of active journals. Convert to alloc_workqueue() and use the default concurrency level so that they can be processed in parallel. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: reiserfs-devel@vger.kernel.org --- Only compile tested. Please feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. fs/reiserfs/journal.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/fs/reiserfs/journal.c b/fs/reiserfs/journal.c index d31bce1..ee311c0 100644 --- a/fs/reiserfs/journal.c +++ b/fs/reiserfs/journal.c @@ -2883,7 +2883,7 @@ int journal_init(struct super_block *sb, const char *j_dev_name, reiserfs_mounted_fs_count++; if (reiserfs_mounted_fs_count <= 1) { reiserfs_write_unlock(sb); - commit_wq = create_workqueue("reiserfs"); + commit_wq = alloc_workqueue("reiserfs", WQ_MEM_RECLAIM, 0); reiserfs_write_lock(sb); } -- 1.7.1 --
Switch to new workqueue interface alloc_workqueue(). These are identity conversions. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Jayamohan Kallickal <jayamohank@serverengines.com> Cc: Andrew Vasquez <andrew.vasquez@qlogic.com> Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com> Cc: linux-scsi@vger.kernel.org --- Please feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. drivers/scsi/be2iscsi/be_main.c | 2 +- drivers/scsi/qla2xxx/qla_os.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c index 75a85aa..4339196 100644 --- a/drivers/scsi/be2iscsi/be_main.c +++ b/drivers/scsi/be2iscsi/be_main.c @@ -4276,7 +4276,7 @@ static int __devinit beiscsi_dev_probe(struct pci_dev *pcidev, snprintf(phba->wq_name, sizeof(phba->wq_name), "beiscsi_q_irq%u", phba->shost->host_no); - phba->wq = create_workqueue(phba->wq_name); + phba->wq = alloc_workqueue(phba->wq_name, WQ_MEM_RECLAIM, 1); if (!phba->wq) { shost_printk(KERN_ERR, phba->shost, "beiscsi_dev_probe-" "Failed to allocate work queue\n"); diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c index 2c0876c..2cd0a77 100644 --- a/drivers/scsi/qla2xxx/qla_os.c +++ b/drivers/scsi/qla2xxx/qla_os.c @@ -356,7 +356,7 @@ static int qla25xx_setup_mode(struct scsi_qla_host *vha) "Can't create request queue\n"); goto fail; } - ha->wq = create_workqueue("qla2xxx_wq"); + ha->wq = alloc_workqueue("qla2xxx_wq", WQ_MEM_RECLAIM, 1); vha->req = ha->req_q_map[req]; options |= BIT_1; for (ques = 1; ques < ha->max_rsp_queues; ques++) { -- 1.7.1 --
ceph messenger code does a rather complex dancing around multithread workqueue to make sure the same work item isn't executed concurrently on different CPUs. This restriction can be provided by workqueue with WQ_NON_REENTRANT. Make ceph_msgr_wq non-reentrant workqueue with the default concurrency level and remove the QUEUED/BUSY logic. * This removes backoff handling in con_work() but it couldn't reliably block execution of con_work() to begin with - queue_con() can be called after the work started but before BUSY is set. It seems that it was an optimization for a rather cold path and can be safely removed. * The number of concurrent work items is bound by the number of connections and connetions are independent from each other. With the default concurrency level, different connections will be executed independently. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Sage Weil <sage@newdream.net> Cc: ceph-devel@vger.kernel.org --- Only compile tested. I think the dropping of backoff logic is safe but am not completely sure. Please verify it's actually okay. Feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. include/linux/ceph/messenger.h | 5 ---- net/ceph/messenger.c | 46 +-------------------------------------- 2 files changed, 2 insertions(+), 49 deletions(-) diff --git a/include/linux/ceph/messenger.h b/include/linux/ceph/messenger.h index a108b42..c3011be 100644 --- a/include/linux/ceph/messenger.h +++ b/include/linux/ceph/messenger.h @@ -110,17 +110,12 @@ struct ceph_msg_pos { /* * ceph_connection state bit flags - * - * QUEUED and BUSY are used together to ensure that only a single - * thread is currently opening, reading or writing data to the socket. */ #define LOSSYTX 0 /* we can close channel or drop messages on errors */ #define CONNECTING 1 #define NEGOTIATING 2 #define KEEPALIVE_PENDING 3 #define WRITE_PENDING 4 /* we have data ...
Tejun- this is a very welcome simplification! I'll take it through the ceph tree, as I want to test it thoroughly and make sure the backoff change is correct (there was some subtlety there, IIRC). Thanks! --
fsc->*_wq's aren't depended upon during memory reclaim. Convert to alloc_workqueue() w/o WQ_MEM_RECLAIM. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Sage Weil <sage@newdream.net> Cc: ceph-devel@vger.kernel.org --- Only compile tested. Please feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. fs/ceph/super.c | 10 +++++++--- 1 files changed, 7 insertions(+), 3 deletions(-) diff --git a/fs/ceph/super.c b/fs/ceph/super.c index 08b460a..1f6436e 100644 --- a/fs/ceph/super.c +++ b/fs/ceph/super.c @@ -443,13 +443,17 @@ struct ceph_fs_client *create_fs_client(struct ceph_mount_options *fsopt, goto fail_client; err = -ENOMEM; - fsc->wb_wq = create_workqueue("ceph-writeback"); + /* + * The number of concurrent works can be high but they don't need + * to be processed in parallel, limit concurrency. + */ + fsc->wb_wq = alloc_workqueue("ceph-writeback", 0, 1); if (fsc->wb_wq == NULL) goto fail_bdi; - fsc->pg_inv_wq = create_singlethread_workqueue("ceph-pg-invalid"); + fsc->pg_inv_wq = alloc_workqueue("ceph-pg-invalid", 0, 1); if (fsc->pg_inv_wq == NULL) goto fail_wb_wq; - fsc->trunc_wq = create_singlethread_workqueue("ceph-trunc"); + fsc->trunc_wq = alloc_workqueue("ceph-trunc", 0, 1); if (fsc->trunc_wq == NULL) goto fail_pg_inv_wq; -- 1.7.1 --
I'll take this one through the ceph tree as well. Thanks! --
aio_wq isn't used during memory reclaim. Convert to alloc_workqueue() without WQ_MEM_RECLAIM. It's possible to use system_wq but given that the number of work items is determined from userland and the work item may block, enforcing strict concurrency limit would be a good idea. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Benjamin LaHaise <bcrl@kvack.org> Cc: linux-aio@kvack.org --- Please feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. fs/aio.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/aio.c b/fs/aio.c index 8c8f6c5..dc3fcbb 100644 --- a/fs/aio.c +++ b/fs/aio.c @@ -85,7 +85,7 @@ static int __init aio_setup(void) kiocb_cachep = KMEM_CACHE(kiocb, SLAB_HWCACHE_ALIGN|SLAB_PANIC); kioctx_cachep = KMEM_CACHE(kioctx,SLAB_HWCACHE_ALIGN|SLAB_PANIC); - aio_wq = create_workqueue("aio"); + aio_wq = alloc_workqueue("aio", 0, 1); /* used to limit concurrency */ abe_pool = mempool_create_kmalloc_pool(1, sizeof(struct aio_batch_entry)); BUG_ON(!abe_pool); @@ -569,7 +569,7 @@ static int __aio_put_req(struct kioctx *ctx, struct kiocb *req) spin_lock(&fput_lock); list_add(&req->ki_list, &fput_head); spin_unlock(&fput_lock); - queue_work(aio_wq, &fput_work); + schedule_work(&fput_work); } else { req->ki_filp = NULL; really_put_req(ctx, req); -- 1.7.1 --
I would think that just given that it may block would be enough to keep OK, the only difference here is the removal of the WQ_MEM_RECLAIM flag, I'm not sure where this change fits into the patch description. Why did you do this? Cheers, Jeff --
With cmwq, there's no reason to use separate workqueues. Drop msmfb_info->resume_workqueue and use system_wq instead. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Stanislaw Gruszka <stf_xl@wp.pl> Cc: linux-usb@vger.kernel.org --- Only compile tested. Please feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. drivers/video/msm/msm_fb.c | 11 +---------- 1 files changed, 1 insertions(+), 10 deletions(-) diff --git a/drivers/video/msm/msm_fb.c b/drivers/video/msm/msm_fb.c index debe593..5436aeb 100644 --- a/drivers/video/msm/msm_fb.c +++ b/drivers/video/msm/msm_fb.c @@ -81,7 +81,6 @@ struct msmfb_info { spinlock_t update_lock; struct mutex panel_init_lock; wait_queue_head_t frame_wq; - struct workqueue_struct *resume_workqueue; struct work_struct resume_work; struct msmfb_callback dma_callback; struct msmfb_callback vsync_callback; @@ -111,7 +110,7 @@ static void msmfb_handle_dma_interrupt(struct msmfb_callback *callback) if (msmfb->sleeping == UPDATING && msmfb->frame_done == msmfb->update_frame) { DLOG(SUSPEND_RESUME, "full update completed\n"); - queue_work(msmfb->resume_workqueue, &msmfb->resume_work); + schedule_work(&msmfb->resume_work); } spin_unlock_irqrestore(&msmfb->update_lock, irq_flags); wake_up(&msmfb->frame_wq); @@ -559,12 +558,6 @@ static int msmfb_probe(struct platform_device *pdev) spin_lock_init(&msmfb->update_lock); mutex_init(&msmfb->panel_init_lock); init_waitqueue_head(&msmfb->frame_wq); - msmfb->resume_workqueue = create_workqueue("panel_on"); - if (msmfb->resume_workqueue == NULL) { - printk(KERN_ERR "failed to create panel_on workqueue\n"); - ret = -ENOMEM; - goto error_create_workqueue; - } INIT_WORK(&msmfb->resume_work, power_on_panel); msmfb->black = kzalloc(msmfb->fb->var.bits_per_pixel*msmfb->xres, GFP_KERNEL); @@ -589,8 +582,6 @@ static int msmfb_probe(struct platform_device *pdev) return 0; ...
ARM/QUALCOMM MSM MACHINE SUPPORT M: David Brown <davidb@codeaurora.org> M: Daniel Walker <dwalker@codeaurora.org> M: Bryan Huntsman <bryanh@codeaurora.org> L: linux-arm-msm@vger.kernel.org F: arch/arm/mach-msm/ F: drivers/video/msm/ F: drivers/mmc/host/msm_sdcc.c F: drivers/mmc/host/msm_sdcc.h F: drivers/serial/msm_serial.h F: drivers/serial/msm_serial.c T: git git://codeaurora.org/quic/kernel/davidb/linux-msm.git S: Maintained -- Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. --
With cmwq, there's no reason to use separate workqueues. Drop uea_softc->work_q and use system_wq instead. The used work item is sync flushed on driver detach. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Stanislaw Gruszka <stf_xl@wp.pl> Cc: linux-usb@vger.kernel.org --- Only compile tested. Please feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. drivers/usb/atm/ueagle-atm.c | 19 +++++-------------- 1 files changed, 5 insertions(+), 14 deletions(-) diff --git a/drivers/usb/atm/ueagle-atm.c b/drivers/usb/atm/ueagle-atm.c index 44447f5..55c1d3b 100644 --- a/drivers/usb/atm/ueagle-atm.c +++ b/drivers/usb/atm/ueagle-atm.c @@ -168,7 +168,6 @@ struct uea_softc { union cmv_dsc cmv_dsc; struct work_struct task; - struct workqueue_struct *work_q; u16 pageno; u16 ovl; @@ -1879,7 +1878,7 @@ static int uea_start_reset(struct uea_softc *sc) /* start loading DSP */ sc->pageno = 0; sc->ovl = 0; - queue_work(sc->work_q, &sc->task); + schedule_work(&sc->task); /* wait for modem ready CMV */ ret = wait_cmv_ack(sc); @@ -2091,14 +2090,14 @@ static void uea_schedule_load_page_e1(struct uea_softc *sc, { sc->pageno = intr->e1_bSwapPageNo; sc->ovl = intr->e1_bOvl >> 4 | intr->e1_bOvl << 4; - queue_work(sc->work_q, &sc->task); + schedule_work(&sc->task); } static void uea_schedule_load_page_e4(struct uea_softc *sc, struct intr_pkt *intr) { sc->pageno = intr->e4_bSwapPageNo; - queue_work(sc->work_q, &sc->task); + schedule_work(&sc->task); } /* @@ -2170,13 +2169,6 @@ static int uea_boot(struct uea_softc *sc) init_waitqueue_head(&sc->sync_q); - sc->work_q = create_workqueue("ueagle-dsp"); - if (!sc->work_q) { - uea_err(INS_TO_USBDEV(sc), "cannot allocate workqueue\n"); - uea_leaves(INS_TO_USBDEV(sc)); - return -ENOMEM; - } - if (UEA_CHIP_VERSION(sc) == ADI930) load_XILINX_firmware(sc); @@ -2222,7 +2214,6 @@ err1: sc->urb_int = NULL; ...
Workqueue scsi_tgtd isn't used during memory reclaim. Convert to alloc_workqueue() without WQ_MEM_RECLAIM. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: FUJITA Tomonori <tomof@acm.org> Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com> Cc: linux-scsi@vger.kernel.org --- Only compile tested. Please feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. drivers/scsi/scsi_tgt_lib.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/scsi/scsi_tgt_lib.c b/drivers/scsi/scsi_tgt_lib.c index c399be9..f672820 100644 --- a/drivers/scsi/scsi_tgt_lib.c +++ b/drivers/scsi/scsi_tgt_lib.c @@ -629,7 +629,7 @@ static int __init scsi_tgt_init(void) if (!scsi_tgt_cmd_cache) return -ENOMEM; - scsi_tgtd = create_workqueue("scsi_tgtd"); + scsi_tgtd = alloc_workqueue("scsi_tgtd", 0, 1); if (!scsi_tgtd) { err = -ENOMEM; goto free_kmemcache; -- 1.7.1 --
With cmwq, there's no reason to use separate workqueues in ipw2x00 drivers. Drop them and use system_wq instead. All used work items are sync canceled on driver detach. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: "John W. Linville" <linville@tuxdriver.com> Cc: linux-wireless@vger.kernel.org --- Only compile tested. Please feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. drivers/net/wireless/ipw2x00/ipw2100.c | 70 +++++------- drivers/net/wireless/ipw2x00/ipw2100.h | 1 - drivers/net/wireless/ipw2x00/ipw2200.c | 196 ++++++++++++++------------------ drivers/net/wireless/ipw2x00/ipw2200.h | 2 - 4 files changed, 118 insertions(+), 151 deletions(-) diff --git a/drivers/net/wireless/ipw2x00/ipw2100.c b/drivers/net/wireless/ipw2x00/ipw2100.c index 61915f3..471a52a 100644 --- a/drivers/net/wireless/ipw2x00/ipw2100.c +++ b/drivers/net/wireless/ipw2x00/ipw2100.c @@ -706,11 +706,10 @@ static void schedule_reset(struct ipw2100_priv *priv) netif_stop_queue(priv->net_dev); priv->status |= STATUS_RESET_PENDING; if (priv->reset_backoff) - queue_delayed_work(priv->workqueue, &priv->reset_work, - priv->reset_backoff * HZ); + schedule_delayed_work(&priv->reset_work, + priv->reset_backoff * HZ); else - queue_delayed_work(priv->workqueue, &priv->reset_work, - 0); + schedule_delayed_work(&priv->reset_work, 0); if (priv->reset_backoff < MAX_RESET_BACKOFF) priv->reset_backoff++; @@ -1474,7 +1473,7 @@ static int ipw2100_enable_adapter(struct ipw2100_priv *priv) if (priv->stop_hang_check) { priv->stop_hang_check = 0; - queue_delayed_work(priv->workqueue, &priv->hang_check, HZ / 2); + schedule_delayed_work(&priv->hang_check, HZ / 2); } fail_up: @@ -1808,8 +1807,8 @@ static int ipw2100_up(struct ipw2100_priv *priv, int deferred) if (priv->stop_rf_kill) { priv->stop_rf_kill = 0; - queue_delayed_work(priv->workqueue, ...
kcrypto_wq and pcrypt->wq's are used to run ciphers and may consume considerable amount of CPU cycles. Mark both as CPU_INTENSIVE so that they don't block other work items. As the workqueues are primarily used to burn CPU cycles, concurrency levels shouldn't matter much and are left at 1. A higher value may be beneficial and needs investigation. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Herbert Xu <herbert@gondor.apana.org.au> Cc: "David S. Miller" <davem@davemloft.net> Cc: linux-crypto@vger.kernel.org --- Only compile tested. Please feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. crypto/crypto_wq.c | 3 ++- crypto/pcrypt.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/crypto/crypto_wq.c b/crypto/crypto_wq.c index fdcf624..b980ee1 100644 --- a/crypto/crypto_wq.c +++ b/crypto/crypto_wq.c @@ -20,7 +20,8 @@ EXPORT_SYMBOL_GPL(kcrypto_wq); static int __init crypto_wq_init(void) { - kcrypto_wq = create_workqueue("crypto"); + kcrypto_wq = alloc_workqueue("crypto", + WQ_MEM_RECLAIM | WQ_CPU_INTENSIVE, 1); if (unlikely(!kcrypto_wq)) return -ENOMEM; return 0; diff --git a/crypto/pcrypt.c b/crypto/pcrypt.c index 75586f1..29a89da 100644 --- a/crypto/pcrypt.c +++ b/crypto/pcrypt.c @@ -455,7 +455,8 @@ static int pcrypt_init_padata(struct padata_pcrypt *pcrypt, get_online_cpus(); - pcrypt->wq = create_workqueue(name); + pcrypt->wq = alloc_workqueue(name, + WQ_MEM_RECLAIM | WQ_CPU_INTENSIVE, 1); if (!pcrypt->wq) goto err; -- 1.7.1 --
Patch applied. Thanks! -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt --
This is an identity conversion.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Markus Lidel <Markus.Lidel@shadowconnect.com>
---
Please feel free to take it into the subsystem tree or simply ack -
I'll route it through the wq tree.
Thanks.
drivers/message/i2o/driver.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/message/i2o/driver.c b/drivers/message/i2o/driver.c
index a0421ef..8a5b2d8 100644
--- a/drivers/message/i2o/driver.c
+++ b/drivers/message/i2o/driver.c
@@ -84,7 +84,8 @@ int i2o_driver_register(struct i2o_driver *drv)
osm_debug("Register driver %s\n", drv->name);
if (drv->event) {
- drv->event_queue = create_workqueue(drv->name);
+ drv->event_queue = alloc_workqueue(drv->name,
+ WQ_MEM_RECLAIM, 1);
if (!drv->event_queue) {
osm_err("Could not initialize event queue for driver "
"%s\n", drv->name);
--
1.7.1
--
With cmwq, there's no reason to use separate out_work_queue. Drop it and use system_wq instead. The in_work_queue needs to be ordered so can't use one of the system wqs; however, as it isn't used to reclaim memory, allocate the workqueue with alloc_ordered_workqueue() without WQ_MEM_RECLAIM. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Andy Walls <awalls@md.metrocast.net> Cc: linux-media@vger.kernel.org --- Only compile tested. Please feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. drivers/media/video/cx18/cx18-driver.c | 24 ++---------------------- drivers/media/video/cx18/cx18-driver.h | 3 --- drivers/media/video/cx18/cx18-streams.h | 3 +-- 3 files changed, 3 insertions(+), 27 deletions(-) diff --git a/drivers/media/video/cx18/cx18-driver.c b/drivers/media/video/cx18/cx18-driver.c index df60f27..41c0822 100644 --- a/drivers/media/video/cx18/cx18-driver.c +++ b/drivers/media/video/cx18/cx18-driver.c @@ -656,7 +656,7 @@ static int __devinit cx18_create_in_workq(struct cx18 *cx) { snprintf(cx->in_workq_name, sizeof(cx->in_workq_name), "%s-in", cx->v4l2_dev.name); - cx->in_work_queue = create_singlethread_workqueue(cx->in_workq_name); + cx->in_work_queue = alloc_ordered_workqueue(cx->in_workq_name, 0); if (cx->in_work_queue == NULL) { CX18_ERR("Unable to create incoming mailbox handler thread\n"); return -ENOMEM; @@ -664,18 +664,6 @@ static int __devinit cx18_create_in_workq(struct cx18 *cx) return 0; } -static int __devinit cx18_create_out_workq(struct cx18 *cx) -{ - snprintf(cx->out_workq_name, sizeof(cx->out_workq_name), "%s-out", - cx->v4l2_dev.name); - cx->out_work_queue = create_workqueue(cx->out_workq_name); - if (cx->out_work_queue == NULL) { - CX18_ERR("Unable to create outgoing mailbox handler threads\n"); - return -ENOMEM; - } - return 0; -} - static void __devinit cx18_init_in_work_orders(struct cx18 *cx) { int i; @@ -702,15 +690,9 @@ static int ...
Tejun, I have a question about cmwq, but to ask it, I need to give the context. The non-system out_work_queue in cx18 had two purposes: 1. To prevent a userspace video rendering application from sleeping in read(), when it had emptied a cx18 driver DMA buffer and the cx18 driver had to sleep to notify the CX2318 engine that the buffer was available again. Your change has no adverse effect on this. 2. To prevent work items being handled by keventd/n from being delayed too long, as the deferred work in question can involve a bit of sleeping due to contention, the workload of the CX23418's MPEG encoding engine, and the number of CX23418 devices in the system. Will all the sleeping that can happen, is the move to a system wq, under cmwq, going to have adverse affects on processing other work items in the system? I get the feeling it won't be a problem with cmwq, but I haven't paid enough attention to be sure. Here are some gory details, if it helps: A single CX23418 has *one* cx18 driver to CX23418 Capture Processing Unit (epu2cpu_mb) mailbox though which the cx18 driver sends all normal commands along with a notification interrupt to the CX23418. The CX23418 will acknowledge all commands with ack back in the mailbox and an interrupt for the cx18 driver. Sometimes the ack happens right away. Sometimes the cx18 driver has to wait(), if the CX23418 is busy. The cx18 driver uses a mutex (epu2cpu_mb_lock) to protect cx18 driver access to the one epu2cpu_mb mailbox. The mutex is grabbed and released on a per epu2cpu_mb mailbox command basis To tell the CX23418 about a usable empty buffer for DMA, one epu2cpu mailbox command (CX18_CPU_DE_SET_MDL) must be performed for each usable empty buffer. A CX23418 can support multiple, simultaneous logical capture streams at once. The streams for a DVB TS, analog video converted to MPEG, VBI data, and the MPEG Index data are common to have active simultaneously. The CX23418 can keep track of 63 empty buffers ...
Hello, It won't be a problem. Now the system_wq supports parallel execution of multiple works and manages concurrency automatically. Work items can sleep as necessary without worrying about other work items. IIUC, if they spend any significant amount of time executing, they'll be doing so by waiting for events (mutex, IRQ...), right? If so, there's nothing to worry about. If it's gonna burn a lot of CPU cycles, we'll need to use a workqueue marked CPU_INTENSIVE but I don't think that's the case here. Thank you. -- tejun --
OK, that's what I thought. I just wanted to ensure that busy CX23418 chips are not going to delay the processing of other work-events in the Right. As a CX23418 is expected to do more streaming by the host, the mailbox mutex contention goes up and the latency of the CX23418 Thanks. I'll do a proper inspection of the patch tonight. It looked OK on cursory review. Regards, Andy --
With cmwq, there's no reason to use a separate workqueue. Drop tps6507x_ts->wq and use system_wq instead. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> Cc: linux-input@vger.kernel.org Cc: Dan Carpenter <error27@gmail.com> Cc: Todd Fischer <todd.fischer@ridgerun.com> --- Only compile tested. Please feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. drivers/input/touchscreen/tps6507x-ts.c | 12 ++++-------- 1 files changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/input/touchscreen/tps6507x-ts.c b/drivers/input/touchscreen/tps6507x-ts.c index c8c136c..4303149 100644 --- a/drivers/input/touchscreen/tps6507x-ts.c +++ b/drivers/input/touchscreen/tps6507x-ts.c @@ -43,7 +43,6 @@ struct tps6507x_ts { struct input_dev *input_dev; struct device *dev; char phys[32]; - struct workqueue_struct *wq; struct delayed_work work; unsigned polling; /* polling is active */ struct ts_event tc; @@ -220,8 +219,8 @@ done: poll = 1; if (poll) { - schd = queue_delayed_work(tsc->wq, &tsc->work, - msecs_to_jiffies(tsc->poll_period)); + schd = schedule_delayed_work(&tsc->work, + msecs_to_jiffies(tsc->poll_period)); if (schd) tsc->polling = 1; else { @@ -303,7 +302,6 @@ static int tps6507x_ts_probe(struct platform_device *pdev) tsc->input_dev = input_dev; INIT_DELAYED_WORK(&tsc->work, tps6507x_ts_handler); - tsc->wq = create_workqueue("TPS6507x Touchscreen"); if (init_data) { tsc->poll_period = init_data->poll_period; @@ -325,8 +323,8 @@ static int tps6507x_ts_probe(struct platform_device *pdev) if (error) goto err2; - schd = queue_delayed_work(tsc->wq, &tsc->work, - msecs_to_jiffies(tsc->poll_period)); + schd = schedule_delayed_work(&tsc->work, + msecs_to_jiffies(tsc->poll_period)); if (schd) tsc->polling = 1; @@ -341,7 +339,6 @@ static int tps6507x_ts_probe(struct platform_device ...
This is that driver that polls every 30 ms. :/ Todd, is there no way to avoid this? What was the status on that? regards, dan carpenter --
Hi Dan, Polling is only necessary because I was given brain dead hardware. I was expecting updated hardware with the interrupt signal connected properly, but haven't received it yet. Any reasonable hardware will not need tps6507x-ts polling. I was under the impression I could only push patches that I have tested. Since I haven't tested an interrupt version of the driver, I didn't include a patch that removes the polling and replaces it with interrupts. I have another customer planning on using the TPS6507x chip for touch screen. I expect their hardware in around 3 weeks. Todd --
With cmwq, there's no reason for radeon to use a dedicated workqueue. Drop dev_priv->wq and use system_wq instead. Because radeon_driver_irq_uninstall_kms() may be called from unsleepable context, the work items can't be flushed from there. Instead, init and flush from radeon_irq_kms_init/fini(). While at it, simplify canceling/flushing of rdev->pm.dynpm_idle_work. Always initialize and sync cancel instead of being unnecessarily smart about it. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: David Airlie <airlied@linux.ie> Cc: dri-devel@lists.freedesktop.org --- Only compile tested. Please feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. drivers/gpu/drm/radeon/evergreen.c | 2 +- drivers/gpu/drm/radeon/r100.c | 2 +- drivers/gpu/drm/radeon/r600.c | 2 +- drivers/gpu/drm/radeon/radeon.h | 1 - drivers/gpu/drm/radeon/radeon_device.c | 6 ---- drivers/gpu/drm/radeon/radeon_irq_kms.c | 5 ++- drivers/gpu/drm/radeon/radeon_pm.c | 47 ++++++++++-------------------- drivers/gpu/drm/radeon/rs600.c | 2 +- 8 files changed, 23 insertions(+), 44 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 7b337c3..6540adb 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -2516,7 +2516,7 @@ restart_ih: if (wptr != rdev->ih.wptr) goto restart_ih; if (queue_hotplug) - queue_work(rdev->wq, &rdev->hotplug_work); + schedule_work(&rdev->hotplug_work); rdev->ih.rptr = rptr; WREG32(IH_RB_RPTR, rdev->ih.rptr); spin_unlock_irqrestore(&rdev->ih.lock, flags); diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c index 8e10aa9..1d15748 100644 --- a/drivers/gpu/drm/radeon/r100.c +++ b/drivers/gpu/drm/radeon/r100.c @@ -622,7 +622,7 @@ int r100_irq_process(struct radeon_device *rdev) /* reset gui idle ack. the status bit is broken */ ...
With cmwq, there's no reason for cpufreq drivers to use separate workqueues. Remove the dedicated workqueues from cpufreq_conservative and cpufreq_ondemand and use system_wq instead. The work items are already sync canceled on stop, so it's already guaranteed that no work is running on module exit. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Dave Jones <davej@redhat.com> Cc: cpufreq@vger.kernel.org --- Only compile tested. Please feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. drivers/cpufreq/cpufreq_conservative.c | 22 +++------------------- drivers/cpufreq/cpufreq_ondemand.c | 20 +++----------------- 2 files changed, 6 insertions(+), 36 deletions(-) diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c index 526bfbf..94284c8 100644 --- a/drivers/cpufreq/cpufreq_conservative.c +++ b/drivers/cpufreq/cpufreq_conservative.c @@ -81,8 +81,6 @@ static unsigned int dbs_enable; /* number of CPUs using this policy */ */ static DEFINE_MUTEX(dbs_mutex); -static struct workqueue_struct *kconservative_wq; - static struct dbs_tuners { unsigned int sampling_rate; unsigned int sampling_down_factor; @@ -560,7 +558,7 @@ static void do_dbs_timer(struct work_struct *work) dbs_check_cpu(dbs_info); - queue_delayed_work_on(cpu, kconservative_wq, &dbs_info->work, delay); + schedule_delayed_work_on(cpu, &dbs_info->work, delay); mutex_unlock(&dbs_info->timer_mutex); } @@ -572,8 +570,7 @@ static inline void dbs_timer_init(struct cpu_dbs_info_s *dbs_info) dbs_info->enable = 1; INIT_DELAYED_WORK_DEFERRABLE(&dbs_info->work, do_dbs_timer); - queue_delayed_work_on(dbs_info->cpu, kconservative_wq, &dbs_info->work, - delay); + schedule_delayed_work_on(dbs_info->cpu, &dbs_info->work, delay); } static inline void dbs_timer_exit(struct cpu_dbs_info_s *dbs_info) @@ -716,25 +713,12 @@ struct cpufreq_governor cpufreq_gov_conservative = { static ...
kblockd is used for unplugging and may affect IO latency and throughput and the max number of concurrent work items are bound by the number of block devices. Make it HIGHPRI workqueue w/ default max concurrency. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Jens Axboe <axboe@kernel.dk> --- Please feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. block/blk-core.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/block/blk-core.c b/block/blk-core.c index 4ce953f..909b84a 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -2606,7 +2606,9 @@ int __init blk_dev_init(void) BUILD_BUG_ON(__REQ_NR_BITS > 8 * sizeof(((struct request *)0)->cmd_flags)); - kblockd_workqueue = create_workqueue("kblockd"); + /* used for unplugging and affects IO latency/throughput - HIGHPRI */ + kblockd_workqueue = alloc_workqueue("kblockd", + WQ_MEM_RECLAIM | WQ_HIGHPRI, 0); if (!kblockd_workqueue) panic("Failed to create kblockd\n"); -- 1.7.1 --
Thanks Tejun, I've pulled the two into the 2.6.38 tree. -- Jens Axboe --
With cmwq, there's no reason to use a separate workqueue in cpufreq_spudemand. Use system_wq instead. The work items are already sync canceled on stop, so it's already guaranteed that no work is running when spu_gov_exit() is entered. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Arnd Bergmann <arnd@arndb.de> Cc: linuxppc-dev@lists.ozlabs.org Cc: Dave Jones <davej@redhat.com> Cc: cpufreq@vger.kernel.org --- Only compile tested. Please feel free to take it into the subsystem tree or simply ack - I'll route it through the wq tree. Thanks. arch/powerpc/platforms/cell/cpufreq_spudemand.c | 20 +++----------------- 1 files changed, 3 insertions(+), 17 deletions(-) diff --git a/arch/powerpc/platforms/cell/cpufreq_spudemand.c b/arch/powerpc/platforms/cell/cpufreq_spudemand.c index 968c1c0..d809836 100644 --- a/arch/powerpc/platforms/cell/cpufreq_spudemand.c +++ b/arch/powerpc/platforms/cell/cpufreq_spudemand.c @@ -39,8 +39,6 @@ struct spu_gov_info_struct { }; static DEFINE_PER_CPU(struct spu_gov_info_struct, spu_gov_info); -static struct workqueue_struct *kspugov_wq; - static int calc_freq(struct spu_gov_info_struct *info) { int cpu; @@ -71,14 +69,14 @@ static void spu_gov_work(struct work_struct *work) __cpufreq_driver_target(info->policy, target_freq, CPUFREQ_RELATION_H); delay = usecs_to_jiffies(info->poll_int); - queue_delayed_work_on(info->policy->cpu, kspugov_wq, &info->work, delay); + schedule_delayed_work_on(info->policy->cpu, &info->work, delay); } static void spu_gov_init_work(struct spu_gov_info_struct *info) { int delay = usecs_to_jiffies(info->poll_int); INIT_DELAYED_WORK_DEFERRABLE(&info->work, spu_gov_work); - queue_delayed_work_on(info->policy->cpu, kspugov_wq, &info->work, delay); + schedule_delayed_work_on(info->policy->cpu, &info->work, delay); } static void spu_gov_cancel_work(struct spu_gov_info_struct *info) @@ -152,27 +150,15 @@ static int __init spu_gov_init(void) { int ret; - kspugov_wq = ...
