From: Lai Jiangshan <laijs@cn.fujitsu.com>
To: <linux-kernel@vger.kernel.org>
Cc: Tejun Heo <tj@kernel.org>, Lai Jiangshan <laijs@cn.fujitsu.com>
Subject: [PATCH 2/3] workqueue: remove the guarantee and the retrying of maybe_create_worker()
Date: Fri, 11 Jul 2014 00:01:04 +0800 [thread overview]
Message-ID: <1405008074-11031-3-git-send-email-laijs@cn.fujitsu.com> (raw)
In-Reply-To: <1405008074-11031-1-git-send-email-laijs@cn.fujitsu.com>
maybe_create_worker() has a strong guarantee that there has at least
one idle worker on return from this function. This guarantee is guaranteed
via the check and the retrying in this function.
But the caller (worker_thread()) also has the same check and retrying,
so the guarantee is not really required and the check and the retrying in
maybe_create_worker() are unnecessary and redundant.
So we remove the guarantee as well as the check and the retrying. The caller
takes responsibility to retry when needed.
The only trade-off is that the mayday timer will be removed and re-added
across retrying. Since retrying is expected as rare case, the trade-off
is acceptible.
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
kernel/workqueue.c | 49 ++++++++++++++++++-------------------------------
1 files changed, 18 insertions(+), 31 deletions(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 92f7ea0c..4fdd6d0 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -1875,14 +1875,17 @@ static void pool_mayday_timeout(unsigned long __pool)
* @pool: pool to create a new worker for
*
* Create a new worker for @pool if necessary. @pool is guaranteed to
- * have at least one idle worker on return from this function. If
- * creating a new worker takes longer than MAYDAY_INTERVAL, mayday is
+ * make some progresses on return from this function.
+ * 1) success to create a new idle worker. Or
+ * 2) cool down a while after it failed. Or
+ * 3) condition changed (no longer need to create worker) after it failed.
+ * In any case, the caller will recheck the condition and retry when needed,
+ * so this function doesn't need to retry.
+ *
+ * If creating a new worker takes longer than MAYDAY_INTERVAL, mayday is
* sent to all rescuers with works scheduled on @pool to resolve
* possible allocation deadlock.
*
- * On return, need_to_create_worker() is guaranteed to be %false and
- * may_start_working() %true.
- *
* LOCKING:
* spin_lock_irq(pool->lock) which may be released and regrabbed
* multiple times. Does GFP_KERNEL allocations. Called only from
@@ -1892,38 +1895,26 @@ static void maybe_create_worker(struct worker_pool *pool)
__releases(&pool->lock)
__acquires(&pool->lock)
{
-restart:
+ struct worker *worker;
+
spin_unlock_irq(&pool->lock);
/* if we don't make progress in MAYDAY_INITIAL_TIMEOUT, call for help */
mod_timer(&pool->mayday_timer, jiffies + MAYDAY_INITIAL_TIMEOUT);
- while (true) {
- struct worker *worker;
-
- worker = create_worker(pool);
- if (worker) {
- del_timer_sync(&pool->mayday_timer);
- spin_lock_irq(&pool->lock);
- start_worker(worker);
- if (WARN_ON_ONCE(need_to_create_worker(pool)))
- goto restart;
- return;
- }
-
- if (!need_to_create_worker(pool))
- break;
+ worker = create_worker(pool);
+ if (worker) {
+ del_timer_sync(&pool->mayday_timer);
+ spin_lock_irq(&pool->lock);
+ start_worker(worker);
+ return;
+ }
+ if (need_to_create_worker(pool))
schedule_timeout_interruptible(CREATE_COOLDOWN);
- if (!need_to_create_worker(pool))
- break;
- }
-
del_timer_sync(&pool->mayday_timer);
spin_lock_irq(&pool->lock);
- if (need_to_create_worker(pool))
- goto restart;
}
/**
@@ -1934,10 +1925,6 @@ restart:
* to. At any given time, there can be only zero or one manager per
* pool. The exclusion is handled automatically by this function.
*
- * The caller can safely start processing works on false return. On
- * true return, it's guaranteed that need_to_create_worker() is false
- * and may_start_working() is true.
- *
* CONTEXT:
* spin_lock_irq(pool->lock) which may be released and regrabbed
* multiple times. Does GFP_KERNEL allocations.
--
1.7.4.4
next prev parent reply other threads:[~2014-07-10 16:00 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-10 16:01 [PATCH 0/3] workqueue: manager cleanup Lai Jiangshan
2014-07-10 16:01 ` [PATCH 1/3] workqueue: remove the first check and the return value of maybe_create_worker() Lai Jiangshan
2014-07-11 15:03 ` Tejun Heo
2014-07-14 2:30 ` Lai Jiangshan
2014-07-14 14:40 ` Tejun Heo
2014-07-10 16:01 ` Lai Jiangshan [this message]
2014-07-11 15:02 ` [PATCH 2/3] workqueue: remove the guarantee and the retrying " Tejun Heo
2014-07-10 16:01 ` [PATCH 3/3] workqueue: unfold start_worker() into create_worker() Lai Jiangshan
2014-07-14 4:05 ` [PATCH 0/1 V2] workqueue: manager cleanup Lai Jiangshan
2014-07-14 4:05 ` [PATCH 1/1 V2] workqueue: unfold start_worker() into create_worker() Lai Jiangshan
2014-07-16 1:28 ` Lai Jiangshan
2014-07-18 22:57 ` Tejun Heo
2014-07-22 5:03 ` [PATCH V3] " Lai Jiangshan
2014-07-22 15:05 ` 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=1405008074-11031-3-git-send-email-laijs@cn.fujitsu.com \
--to=laijs@cn.fujitsu.com \
--cc=linux-kernel@vger.kernel.org \
--cc=tj@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
all inboxes | Powered by JetHome®