linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: linux-kernel@vger.kernel.org
Cc: torvalds@linux-foundation.org, akpm@linux-foundation.org,
	padovan@profusion.mobi, marcel@holtmann.org,
	peterz@infradead.org, mingo@redhat.com, davem@davemloft.net,
	dougthompson@xmission.com, ibm-acpi@hmh.eng.br, cbou@mail.ru,
	rui.zhang@intel.com, tomi.valkeinen@ti.com,
	Tejun Heo <tj@kernel.org>
Subject: [PATCH 11/14] workqueue: reorganize try_to_grab_pending() and __cancel_timer_work()
Date: Fri,  3 Aug 2012 10:43:56 -0700	[thread overview]
Message-ID: <1344015839-21800-12-git-send-email-tj@kernel.org> (raw)
In-Reply-To: <1344015839-21800-1-git-send-email-tj@kernel.org>

* Use bool @is_dwork instead of @timer and let try_to_grab_pending()
  use to_delayed_work() to determine the delayed_work address.

* Move timer handling from __cancel_work_timer() to
  try_to_grab_pending().

* Make try_to_grab_pending() use -EAGAIN instead of -1 for
  busy-looping and drop the ret local variable.

* Add proper function comment to try_to_grab_pending().

This makes the code a bit easier to understand and will ease further
changes.  This patch doesn't make any functional change.

v2: Use @is_dwork instead of @timer.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 kernel/workqueue.c |   47 ++++++++++++++++++++++++++++++++---------------
 1 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index d7f1b7e..4b3663b 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -1004,15 +1004,33 @@ static void cwq_dec_nr_in_flight(struct cpu_workqueue_struct *cwq, int color,
 		complete(&cwq->wq->first_flusher->done);
 }
 
-/*
- * Upon a successful return (>= 0), the caller "owns" WORK_STRUCT_PENDING bit,
- * so this work can't be re-armed in any way.
+/**
+ * try_to_grab_pending - steal work item from worklist
+ * @work: work item to steal
+ * @is_dwork: @work is a delayed_work
+ *
+ * Try to grab PENDING bit of @work.  This function can handle @work in any
+ * stable state - idle, on timer or on worklist.  Return values are
+ *
+ *  1		if @work was pending and we successfully stole PENDING
+ *  0		if @work was idle and we claimed PENDING
+ *  -EAGAIN	if PENDING couldn't be grabbed at the moment, safe to busy-retry
+ *
+ * On >= 0 return, the caller owns @work's PENDING bit.
  */
-static int try_to_grab_pending(struct work_struct *work)
+static int try_to_grab_pending(struct work_struct *work, bool is_dwork)
 {
 	struct global_cwq *gcwq;
-	int ret = -1;
 
+	/* try to steal the timer if it exists */
+	if (is_dwork) {
+		struct delayed_work *dwork = to_delayed_work(work);
+
+		if (likely(del_timer(&dwork->timer)))
+			return 1;
+	}
+
+	/* try to claim PENDING the normal way */
 	if (!test_and_set_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(work)))
 		return 0;
 
@@ -1022,7 +1040,7 @@ static int try_to_grab_pending(struct work_struct *work)
 	 */
 	gcwq = get_work_gcwq(work);
 	if (!gcwq)
-		return ret;
+		return -EAGAIN;
 
 	spin_lock_irq(&gcwq->lock);
 	if (!list_empty(&work->entry)) {
@@ -1038,12 +1056,14 @@ static int try_to_grab_pending(struct work_struct *work)
 			cwq_dec_nr_in_flight(get_work_cwq(work),
 				get_work_color(work),
 				*work_data_bits(work) & WORK_STRUCT_DELAYED);
-			ret = 1;
+
+			spin_unlock_irq(&gcwq->lock);
+			return 1;
 		}
 	}
 	spin_unlock_irq(&gcwq->lock);
 
-	return ret;
+	return -EAGAIN;
 }
 
 /**
@@ -2817,15 +2837,12 @@ bool flush_work_sync(struct work_struct *work)
 }
 EXPORT_SYMBOL_GPL(flush_work_sync);
 
-static bool __cancel_work_timer(struct work_struct *work,
-				struct timer_list* timer)
+static bool __cancel_work_timer(struct work_struct *work, bool is_dwork)
 {
 	int ret;
 
 	do {
-		ret = (timer && likely(del_timer(timer)));
-		if (!ret)
-			ret = try_to_grab_pending(work);
+		ret = try_to_grab_pending(work, is_dwork);
 		wait_on_work(work);
 	} while (unlikely(ret < 0));
 
@@ -2853,7 +2870,7 @@ static bool __cancel_work_timer(struct work_struct *work,
  */
 bool cancel_work_sync(struct work_struct *work)
 {
-	return __cancel_work_timer(work, NULL);
+	return __cancel_work_timer(work, false);
 }
 EXPORT_SYMBOL_GPL(cancel_work_sync);
 
@@ -2914,7 +2931,7 @@ EXPORT_SYMBOL(flush_delayed_work_sync);
  */
 bool cancel_delayed_work_sync(struct delayed_work *dwork)
 {
-	return __cancel_work_timer(&dwork->work, &dwork->timer);
+	return __cancel_work_timer(&dwork->work, true);
 }
 EXPORT_SYMBOL(cancel_delayed_work_sync);
 
-- 
1.7.7.3


  parent reply	other threads:[~2012-08-03 17:45 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-03 17:43 Tejun Heo
2012-08-03 17:43 ` [PATCH 01/14] workqueue: reorder queueing functions so that _on() variants are on top Tejun Heo
2012-08-03 17:43 ` [PATCH 02/14] workqueue: make queueing functions return bool Tejun Heo
2012-08-03 17:43 ` [PATCH 03/14] workqueue: add missing smp_wmb() in process_one_work() Tejun Heo
2012-08-03 17:43 ` [PATCH 04/14] workqueue: disable irq while manipulating PENDING Tejun Heo
2012-08-03 17:43 ` [PATCH 05/14] workqueue: set delayed_work->timer function on initialization Tejun Heo
2012-08-03 17:43 ` [PATCH 06/14] workqueue: unify local CPU queueing handling Tejun Heo
2012-08-03 17:43 ` [PATCH 07/14] workqueue: fix zero @delay handling of queue_delayed_work_on() Tejun Heo
2012-08-03 17:43 ` [PATCH 08/14] workqueue: move try_to_grab_pending() upwards Tejun Heo
2012-08-03 17:43 ` [PATCH 09/14] workqueue: introduce WORK_OFFQ_FLAG_* Tejun Heo
2012-08-03 17:43 ` [PATCH 10/14] workqueue: factor out __queue_delayed_work() from queue_delayed_work_on() Tejun Heo
2012-08-03 17:43 ` Tejun Heo [this message]
2012-08-03 17:43 ` [PATCH 12/14] workqueue: mark a work item being canceled as such Tejun Heo
2012-08-03 17:43 ` [PATCH 13/14] workqueue: implement mod_delayed_work[_on]() Tejun Heo
2012-08-03 17:43 ` [PATCH 14/14] workqueue: use mod_delayed_work() instead of cancel + queue Tejun Heo
2012-08-03 17:45 ` $SUBJ should have been "[PATCHSET] workqueue: implement mod_delayed_work[_on](), take#2" Tejun Heo
2012-08-08 16:39 ` your mail Tejun Heo
2012-08-13 13:27 ` [PATCH 14/14] workqueue: use mod_delayed_work() instead of cancel + queue David Howells

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=1344015839-21800-12-git-send-email-tj@kernel.org \
    --to=tj@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=cbou@mail.ru \
    --cc=davem@davemloft.net \
    --cc=dougthompson@xmission.com \
    --cc=ibm-acpi@hmh.eng.br \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marcel@holtmann.org \
    --cc=mingo@redhat.com \
    --cc=padovan@profusion.mobi \
    --cc=peterz@infradead.org \
    --cc=rui.zhang@intel.com \
    --cc=tomi.valkeinen@ti.com \
    --cc=torvalds@linux-foundation.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).