linux-kernel - [PATCH 2/3] workqueue: remove the guarantee and the retrying of maybe_create

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1405008074-11031-3-git-send-email-laijs@cn.fujitsu.com>
Date:	Fri, 11 Jul 2014 00:01:04 +0800
From:	Lai Jiangshan <laijs@...fujitsu.com>
To:	<linux-kernel@...r.kernel.org>
CC:	Tejun Heo <tj@...nel.org>, Lai Jiangshan <laijs@...fujitsu.com>
Subject: [PATCH 2/3] workqueue: remove the guarantee and the retrying of maybe_create_worker()

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@...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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/