linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Lai Jiangshan <laijs@cn.fujitsu.com>
Cc: linux-kernel@vger.kernel.org
Subject: [PATCH wq/for-3.9 workqueue]: add delayed_work->wq to simplify reentrancy handling
Date: Wed, 6 Feb 2013 16:28:06 -0800	[thread overview]
Message-ID: <20130207002806.GG2875@htj.dyndns.org> (raw)
In-Reply-To: <1359657696-2767-4-git-send-email-laijs@cn.fujitsu.com>

From: Lai Jiangshan <laijs@cn.fujitsu.com>

To avoid executing the same work item from multiple CPUs concurrently,
a work_struct records the last pool it was on in its ->data so that,
on the next queueing, the pool can be queried to determine whether the
work item is still executing or not.

A delayed_work goes through timer before actually being queued on the
target workqueue and the timer needs to know the target workqueue and
CPU.  This is currently achieved by modifying delayed_work->work.data
such that it points to the cwq which points to the target workqueue
and the last CPU the work item was on.  __queue_delayed_work()
extracts the last CPU from delayed_work->work.data and then combines
it with the target workqueue to create new work.data.

The only thing this rather ugly hack achieves is encoding the target
workqueue into delayed_work->work.data without using a separate field,
which could be a trade off one can make; unfortunately, this entangles
work->data management between regular workqueue and delayed_work code
by setting cwq pointer before the work item is actually queued and
becomes a hindrance for further improvements of work->data handling.

This can be easily made sane by adding a target workqueue field to
delayed_work.  While delayed_work is used widely in the kernel and
this does make it a bit larger (<5%), I think this is the right
trade-off especially given the prospect of much saner handling of
work->data which currently involves quite tricky memory barrier
dancing, and don't expect to see any measureable effect.

Add delayed_work->wq and drop the delayed_work->work.data overloading.

tj: Rewrote the description.

Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
---
 include/linux/workqueue.h |    3 +++
 kernel/workqueue.c        |   32 +++-----------------------------
 2 files changed, 6 insertions(+), 29 deletions(-)

--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -109,6 +109,9 @@ struct work_struct {
 struct delayed_work {
 	struct work_struct work;
 	struct timer_list timer;
+
+	/* target workqueue and CPU ->timer uses to queue ->work */
+	struct workqueue_struct *wq;
 	int cpu;
 };
 
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -1339,10 +1339,9 @@ EXPORT_SYMBOL_GPL(queue_work);
 void delayed_work_timer_fn(unsigned long __data)
 {
 	struct delayed_work *dwork = (struct delayed_work *)__data;
-	struct cpu_workqueue_struct *cwq = get_work_cwq(&dwork->work);
 
 	/* should have been called from irqsafe timer with irq already off */
-	__queue_work(dwork->cpu, cwq->wq, &dwork->work);
+	__queue_work(dwork->cpu, dwork->wq, &dwork->work);
 }
 EXPORT_SYMBOL_GPL(delayed_work_timer_fn);
 
@@ -1351,7 +1350,6 @@ static void __queue_delayed_work(int cpu
 {
 	struct timer_list *timer = &dwork->timer;
 	struct work_struct *work = &dwork->work;
-	unsigned int lcpu;
 
 	WARN_ON_ONCE(timer->function != delayed_work_timer_fn ||
 		     timer->data != (unsigned long)dwork);
@@ -1371,30 +1369,7 @@ static void __queue_delayed_work(int cpu
 
 	timer_stats_timer_set_start_info(&dwork->timer);
 
-	/*
-	 * This stores cwq for the moment, for the timer_fn.  Note that the
-	 * work's pool is preserved to allow reentrance detection for
-	 * delayed works.
-	 */
-	if (!(wq->flags & WQ_UNBOUND)) {
-		struct worker_pool *pool = get_work_pool(work);
-
-		/*
-		 * If we cannot get the last pool from @work directly,
-		 * select the last CPU such that it avoids unnecessarily
-		 * triggering non-reentrancy check in __queue_work().
-		 */
-		lcpu = cpu;
-		if (pool)
-			lcpu = pool->cpu;
-		if (lcpu == WORK_CPU_UNBOUND)
-			lcpu = raw_smp_processor_id();
-	} else {
-		lcpu = WORK_CPU_UNBOUND;
-	}
-
-	set_work_cwq(work, get_cwq(lcpu, wq), 0);
-
+	dwork->wq = wq;
 	dwork->cpu = cpu;
 	timer->expires = jiffies + delay;
 
@@ -2944,8 +2919,7 @@ bool flush_delayed_work(struct delayed_w
 {
 	local_irq_disable();
 	if (del_timer_sync(&dwork->timer))
-		__queue_work(dwork->cpu,
-			     get_work_cwq(&dwork->work)->wq, &dwork->work);
+		__queue_work(dwork->cpu, dwork->wq, &dwork->work);
 	local_irq_enable();
 	return flush_work(&dwork->work);
 }

  parent reply	other threads:[~2013-02-07  0:28 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-31 18:41 [PATCH 00/13] workqueue: enhance locking and record global worker id for work data Lai Jiangshan
2013-01-31 18:41 ` [PATCH 01/13] workqueue: remove WORK_CPU_NONE Lai Jiangshan
2013-02-04 19:49   ` Tejun Heo
2013-02-05 12:03     ` Lai Jiangshan
2013-02-06 22:34   ` [PATCH wq/for-3.9] workqueue: replace WORK_CPU_NONE/LAST with WORK_CPU_END Tejun Heo
2013-01-31 18:41 ` [PATCH 02/13] workqueue: fix work_busy() Lai Jiangshan
2013-02-04 19:54   ` Tejun Heo
2013-02-05 12:06     ` Lai Jiangshan
2013-02-05 18:53       ` Tejun Heo
2013-02-06 11:42         ` Lai Jiangshan
2013-02-06 14:05           ` Tejun Heo
2013-02-06 23:02   ` [PATCH wq/for-3.9] workqueue: make work_busy() test WORK_STRUCT_PENDING first Tejun Heo
2013-01-31 18:41 ` [PATCH 03/13] workqueue: don't set work cwq until we queued it on pool Lai Jiangshan
2013-02-04 21:28   ` Tejun Heo
2013-02-05 15:00     ` Lai Jiangshan
2013-02-05 16:45       ` Tejun Heo
2013-02-06 11:35         ` Lai Jiangshan
2013-02-06 14:18           ` Tejun Heo
2013-02-05 15:06     ` Lai Jiangshan
2013-02-07  0:28   ` Tejun Heo [this message]
2013-01-31 18:41 ` [PATCH 04/13] workqueue: clear cwq when cancel the work Lai Jiangshan
2013-02-07  0:53   ` [PATCH wq/for-3.9] workqueue: make work->data point to pool after try_to_grab_pending() Tejun Heo
2013-01-31 18:41 ` [PATCH 05/13] workqueue: change queued detection and remove *mb()s Lai Jiangshan
2013-02-07  1:52   ` [PATCH wq/for-3.9] workqueue: simplify is-work-item-queued-here test Tejun Heo
2013-02-07  2:03     ` [PATCH wq/for-3.9] workqueue: cosmetic update in try_to_grab_pending() Tejun Heo
2013-01-31 18:41 ` [PATCH 06/13] workqueue: get pool id from work->data directly if it is offq Lai Jiangshan
2013-02-07 20:16   ` [PATCH wq/for-3.9] workqueue: make get_work_pool_id() cheaper Tejun Heo
2013-01-31 18:41 ` [PATCH 07/13] workqueue: get pool from wq/cwq Lai Jiangshan
2013-02-07 21:13   ` [PATCH wq/for-3.9] workqueue: pick cwq instead of pool in __queue_work() Tejun Heo
2013-01-31 18:41 ` [PATCH 08/13] workqueue: add lock_pool_executing_work() Lai Jiangshan
2013-02-04 21:34   ` Tejun Heo
2013-02-05 12:15     ` Lai Jiangshan
2013-02-05 19:09       ` Tejun Heo
2013-01-31 18:41 ` [PATCH 09/13] workqueue: add lock_pool_queued_work() Lai Jiangshan
2013-01-31 18:41 ` [PATCH 10/13] workqueue: add lock_pool_own_work() and remove get_work_pool() Lai Jiangshan
2013-02-04 21:38   ` Tejun Heo
2013-01-31 18:41 ` [PATCH 11/13] workqueue: allow more work_pool id space Lai Jiangshan
2013-01-31 18:41 ` [PATCH 12/13] workqueue: add worker's global worker ID Lai Jiangshan
2013-02-04 21:39   ` Tejun Heo
2013-01-31 18:41 ` [PATCH 13/13] workqueue: record global worker ID instead of pool ID in work->data when off-queue Lai Jiangshan
2013-02-04 21:40   ` Tejun Heo
2013-02-04 22:12   ` Tejun Heo
2013-02-05 15:18     ` Lai Jiangshan
2013-02-07 22:02   ` Tejun Heo
2013-02-13 22:23     ` Tejun Heo
2013-02-04 21:04 ` [PATCH 00/13] workqueue: enhance locking and record global worker id for work data Tejun Heo
2013-02-05 16:19   ` Lai Jiangshan
2013-02-05 16:50     ` Tejun Heo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130207002806.GG2875@htj.dyndns.org \
    --to=tj@kernel.org \
    --cc=laijs@cn.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).