linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Nigel Cunningham <nigel@nigelcunningham.com.au>,
	"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	Jens Axboe <axboe@kernel.dk>,
	tomaz.solc@tablix.org, aaron.lu@intel.com,
	linux-kernel@vger.kernel.org, Oleg Nesterov <oleg@redhat.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Fengguang Wu <fengguang.wu@intel.com>,
	Lai Jiangshan <laijs@cn.fujitsu.com>,
	David Howells <dhowells@redhat.com>
Subject: [PATCH wq/for-3.14 2/2] workqueue: implement @drain for workqueue_set_max_active()
Date: Thu, 19 Dec 2013 18:37:20 -0500	[thread overview]
Message-ID: <20131219233720.GE22725@htj.dyndns.org> (raw)
In-Reply-To: <1735137.dNmBz0IqxU@vostro.rjw.lan>

Hello, Rafael.

If this looks good to you, I'll commit it to wq/for-3.14 and we can at
least start to clean up things.

Thanks!

------- 8< -------
>From 6b5182f3f193d0ff9296f53a4665a55b6477aa77 Mon Sep 17 00:00:00 2001
From: Tejun Heo <tj@kernel.org>
Date: Thu, 19 Dec 2013 18:33:12 -0500

workqueue_set_max_active() currently doesn't wait for the number of
in-flight work items to fall under the new @max_active.  This patch
adds @drain paramter to workqueue_set_max_active(), if set, the
function sleeps until nr_active on each pool_workqueue of the target
workqueue drops below the current saved_max_active.

This is planned to replace freezable workqueues.  It is determined
that kernel freezables - both kthreads and workqueues - aren't
necessary and just add to the confusion and unnecessary deadlocks.
There are only a handful which actually need to stop processing for
system power events and they can be converted to use
workqueue_set_max_active(WQ_FROZEN_ACTIVE) instead of freezable
workqueues.  Ultimately, all other uses of freezables will be dropped
and the whole system freezer machinery will be excised.  Well, that's
the plan anyway.

The implementation is fairly straight-forward.  As this is expected to
be used by only a handful and most likely not concurrently, a single
wait queue is used.  set_max_active drainers wait on it as necessary
and pwq_dec_nr_in_flight() triggers it if nr_active == max_active
after nr_active is decremented.  This unfortunately adds on unlikely
branch to the work item execution path but this is extremely unlikely
to be noticeable and I think it's worthwhile to avoid polling here as
there may be multiple calls to this function in succession during
suspend and some platforms use suspend quite frequently.

Signed-off-by: Tejun Heo <tj@kernel.org>
Link: http://lkml.kernel.org/g/20131218213936.GA8218@mtj.dyndns.org
Cc: Lai Jiangshan <laijs@cn.fujitsu.com>
Cc: David Howells <dhowells@redhat.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
---
 fs/fscache/main.c         |  2 +-
 include/linux/workqueue.h |  2 +-
 kernel/workqueue.c        | 58 ++++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 57 insertions(+), 5 deletions(-)

diff --git a/fs/fscache/main.c b/fs/fscache/main.c
index 9d5a716..e2837f2 100644
--- a/fs/fscache/main.c
+++ b/fs/fscache/main.c
@@ -67,7 +67,7 @@ static int fscache_max_active_sysctl(struct ctl_table *table, int write,
 	if (*datap < 1)
 		return -EINVAL;
 
-	workqueue_set_max_active(*wqp, *datap);
+	workqueue_set_max_active(*wqp, *datap, false);
 	return 0;
 }
 
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index 334daa3..8b9c628 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -488,7 +488,7 @@ extern bool cancel_delayed_work(struct delayed_work *dwork);
 extern bool cancel_delayed_work_sync(struct delayed_work *dwork);
 
 extern void workqueue_set_max_active(struct workqueue_struct *wq,
-				     int max_active);
+				     int max_active, bool drain);
 extern bool current_is_workqueue_rescuer(void);
 extern bool workqueue_congested(int cpu, struct workqueue_struct *wq);
 extern unsigned int work_busy(struct work_struct *work);
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 6748fbf..f18c35b 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -293,6 +293,12 @@ static DEFINE_SPINLOCK(wq_mayday_lock);	/* protects wq->maydays list */
 static LIST_HEAD(workqueues);		/* PL: list of all workqueues */
 static bool workqueue_freezing;		/* PL: have wqs started freezing? */
 
+/*
+ * Wait for nr_active to drain after max_active adjustment.  This is a cold
+ * path and not expected to have many users.  A global waitq should do.
+ */
+static DECLARE_WAIT_QUEUE_HEAD(wq_nr_active_drain_waitq);
+
 /* the per-cpu worker pools */
 static DEFINE_PER_CPU_SHARED_ALIGNED(struct worker_pool [NR_STD_WORKER_POOLS],
 				     cpu_worker_pools);
@@ -1123,6 +1129,14 @@ static void pwq_dec_nr_in_flight(struct pool_workqueue *pwq, int color)
 	pwq->nr_in_flight[color]--;
 
 	pwq->nr_active--;
+
+	/*
+	 * This can happen only after max_active is lowered.  Tell the
+	 * waiters that draining might be complete.
+	 */
+	if (unlikely(pwq->nr_active == pwq->max_active))
+		wake_up_all(&wq_nr_active_drain_waitq);
+
 	if (!list_empty(&pwq->delayed_works)) {
 		/* one down, submit a delayed one */
 		if (pwq->nr_active < pwq->max_active)
@@ -3140,7 +3154,7 @@ static ssize_t max_active_store(struct device *dev,
 	if (sscanf(buf, "%d", &val) != 1 || val <= 0)
 		return -EINVAL;
 
-	workqueue_set_max_active(wq, val);
+	workqueue_set_max_active(wq, val, false);
 	return count;
 }
 static DEVICE_ATTR_RW(max_active);
@@ -4339,16 +4353,22 @@ EXPORT_SYMBOL_GPL(destroy_workqueue);
  * workqueue_set_max_active - adjust max_active of a workqueue
  * @wq: target workqueue
  * @max_active: new max_active value.
+ * @drain: wait until the actual level of concurrency becomes <= @max_active
  *
- * Set max_active of @wq to @max_active.
+ * Set max_active of @wq to @max_active.  If @drain is true, wait until the
+ * in-flight work items are drained to the new level.
  *
  * CONTEXT:
  * Don't call from IRQ context.
  */
-void workqueue_set_max_active(struct workqueue_struct *wq, int max_active)
+void workqueue_set_max_active(struct workqueue_struct *wq, int max_active,
+			      bool drain)
 {
+	DEFINE_WAIT(wait);
 	struct pool_workqueue *pwq;
 
+	might_sleep_if(drain);
+
 	/* disallow meddling with max_active for ordered workqueues */
 	if (WARN_ON(wq->flags & __WQ_ORDERED))
 		return;
@@ -4363,6 +4383,38 @@ void workqueue_set_max_active(struct workqueue_struct *wq, int max_active)
 		pwq_adjust_max_active(pwq);
 
 	mutex_unlock(&wq->mutex);
+
+	/*
+	 * If we have increased max_active, pwq_dec_nr_in_flight() might
+	 * not trigger for other instances of this function waiting for
+	 * drain.  Force them to check.
+	 */
+	wake_up_all(&wq_nr_active_drain_waitq);
+
+	if (!drain)
+		return;
+
+	/* let's wait for the drain to complete */
+restart:
+	mutex_lock(&wq->mutex);
+	prepare_to_wait(&wq_nr_active_drain_waitq, &wait, TASK_UNINTERRUPTIBLE);
+
+	for_each_pwq(pwq, wq) {
+		/*
+		 * nr_active should be monotonously decreasing as long as
+		 * it's over max_active, so no need to grab pool lock.
+		 * Also, test against saved_max_active in case multiple
+		 * instances and/or system freezer are racing.
+		 */
+		if (pwq->nr_active > wq->saved_max_active) {
+			mutex_unlock(&wq->mutex);
+			schedule();
+			goto restart;
+		}
+	}
+
+	finish_wait(&wq_nr_active_drain_waitq, &wait);
+	mutex_unlock(&wq->mutex);
 }
 EXPORT_SYMBOL_GPL(workqueue_set_max_active);
 
-- 
1.8.4.2


  parent reply	other threads:[~2013-12-19 23:37 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-13 17:49 Writeback threads and freezable Tejun Heo
2013-12-13 18:52 ` Tejun Heo
2013-12-13 20:40   ` [PATCH] libata, freezer: avoid block device removal while system is frozen Tejun Heo
2013-12-13 22:45     ` Nigel Cunningham
2013-12-13 23:07       ` Tejun Heo
2013-12-13 23:15         ` Nigel Cunningham
2013-12-14  1:55           ` Dave Chinner
2013-12-14 20:31           ` Tejun Heo
2013-12-14 20:36             ` Tejun Heo
2013-12-14 21:21               ` Nigel Cunningham
2013-12-17  2:35                 ` Rafael J. Wysocki
2013-12-17  2:34             ` Rafael J. Wysocki
2013-12-17 12:34               ` Tejun Heo
2013-12-18  0:35                 ` Rafael J. Wysocki
2013-12-18 11:17                   ` Tejun Heo
2013-12-18 21:48                     ` Rafael J. Wysocki
2013-12-18 21:39                       ` Tejun Heo
2013-12-18 21:41                         ` Tejun Heo
2013-12-18 22:04                           ` Rafael J. Wysocki
2013-12-19 23:35                             ` [PATCH wq/for-3.14 1/2] workqueue: update max_active clamping rules Tejun Heo
2013-12-20  1:26                               ` Rafael J. Wysocki
2013-12-19 23:37                             ` Tejun Heo [this message]
2013-12-20  1:31                               ` [PATCH wq/for-3.14 2/2] workqueue: implement @drain for workqueue_set_max_active() Rafael J. Wysocki
2013-12-20 13:32                                 ` Tejun Heo
2013-12-20 13:56                                   ` Rafael J. Wysocki
2013-12-20 14:23                                     ` Tejun Heo
2013-12-16 12:12         ` [PATCH] libata, freezer: avoid block device removal while system is frozen Ming Lei
2013-12-16 12:45           ` Tejun Heo
2013-12-16 13:24             ` Ming Lei
2013-12-16 16:05               ` Tejun Heo
2013-12-17  2:38     ` Rafael J. Wysocki
2013-12-17 12:36       ` Tejun Heo
2013-12-18  0:23         ` Rafael J. Wysocki
2013-12-17 12:50     ` [PATCH v2] " Tejun Heo
2013-12-18  1:04       ` Rafael J. Wysocki
2013-12-18 11:08         ` Tejun Heo
2013-12-18 12:07       ` [PATCH v3] " Tejun Heo
2013-12-18 22:08         ` Rafael J. Wysocki
2013-12-19 17:24           ` Tejun Heo
2013-12-19 18:54         ` [PATCH v4] " Tejun Heo
2013-12-14  1:53 ` Writeback threads and freezable Dave Chinner
2013-12-14 17:30   ` Greg Kroah-Hartman
2013-12-14 20:23   ` Tejun Heo
2013-12-16  3:56     ` Dave Chinner
2013-12-16 12:51       ` Tejun Heo
2013-12-16 12:56         ` Tejun Heo
2013-12-18  0:35           ` Dave Chinner
2013-12-18 11:43             ` Tejun Heo
2013-12-18 22:14               ` Rafael J. Wysocki
2013-12-19  4:08               ` Dave Chinner
2013-12-19 16:24                 ` Tejun Heo
2013-12-20  0:51                   ` Dave Chinner
2013-12-20 14:51                     ` Tejun Heo
2013-12-20 14:00                   ` Rafael J. Wysocki

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=20131219233720.GE22725@htj.dyndns.org \
    --to=tj@kernel.org \
    --cc=aaron.lu@intel.com \
    --cc=axboe@kernel.dk \
    --cc=dhowells@redhat.com \
    --cc=fengguang.wu@intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=laijs@cn.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nigel@nigelcunningham.com.au \
    --cc=oleg@redhat.com \
    --cc=rafael.j.wysocki@intel.com \
    --cc=rjw@rjwysocki.net \
    --cc=tomaz.solc@tablix.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).