[PATCH] workqueue: avoid nohz_full cores

[PATCH] workqueue: avoid nohz_full cores

[cmetcalf]

From: Chris Metcalf <cmetcalf@ezchip.com>

When queuing work, we should avoid queuing it on the local cpu if
we are using WORK_CPU_UNBOUND and the local cpu is nohz_full, since
the workqueue will mean a later interrupt of the nohz_full process
that presumably would prefer continuing to have 100% of the core
without interrupts.

Likewise, remove the nohz_full cores from unbound workqueues.  If
all the cores are nohz_full, we leave them in.

Signed-off-by: Chris Metcalf <cmetcalf@ezchip.com>
---
Note that this patch depends on my earlier commit, not yet pulled
into Linus' tree, that added the tick_nohz_full_clear_cpus() API:
https://lkml.org/lkml/2015/3/24/956

 include/linux/tick.h |  9 +++++++++
 kernel/workqueue.c   | 10 +++++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/include/linux/tick.h b/include/linux/tick.h
index 29456c443970..119ed00c96d5 100644
--- a/include/linux/tick.h
+++ b/include/linux/tick.h
@@ -224,6 +224,15 @@ static inline bool is_housekeeping_cpu(int cpu)
 	return true;
 }
 
+static inline int prefer_housekeeping_cpu(int cpu)
+{
+#ifdef CONFIG_NO_HZ_FULL
+	if (tick_nohz_full_cpu(cpu))
+		return cpumask_next(-1, housekeeping_mask);
+#endif
+	return cpu;
+}
+
 static inline void housekeeping_affine(struct task_struct *t)
 {
 #ifdef CONFIG_NO_HZ_FULL
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index f28849394791..ebe5ce3ae42d 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -48,6 +48,7 @@
 #include <linux/nodemask.h>
 #include <linux/moduleparam.h>
 #include <linux/uaccess.h>
+#include <linux/tick.h>
 
 #include "workqueue_internal.h"
 
@@ -1303,7 +1304,7 @@ static void __queue_work(int cpu, struct workqueue_struct *wq,
 		return;
 retry:
 	if (req_cpu == WORK_CPU_UNBOUND)
-		cpu = raw_smp_processor_id();
+		cpu = prefer_housekeeping_cpu(raw_smp_processor_id());
 
 	/* pwq which will be used unless @work is executing elsewhere */
 	if (!(wq->flags & WQ_UNBOUND))
@@ -3782,6 +3783,7 @@ int apply_workqueue_attrs(struct workqueue_struct *wq,
 	/* make a copy of @attrs and sanitize it */
 	copy_workqueue_attrs(new_attrs, attrs);
 	cpumask_and(new_attrs->cpumask, new_attrs->cpumask, cpu_possible_mask);
+	tick_nohz_full_clear_cpus(new_attrs->cpumask);
 
 	/*
 	 * We may create multiple pwqs with differing cpumasks.  Make a
@@ -3810,6 +3812,12 @@ int apply_workqueue_attrs(struct workqueue_struct *wq,
 
 	for_each_node(node) {
 		if (wq_calc_node_cpumask(attrs, node, -1, tmp_attrs->cpumask)) {
+#ifdef CONFIG_NO_HZ_FULL
+			tick_nohz_full_clear_cpus(tmp_attrs->cpumask);
+			if (cpumask_empty(tmp_attrs->cpumask))
+				cpumask_copy(tmp_attrs->cpumask,
+					     new_attrs->cpumask);
+#endif
 			pwq_tbl[node] = alloc_unbound_pwq(wq, tmp_attrs);
 			if (!pwq_tbl[node])
 				goto enomem_pwq;
-- 
2.1.2

Re: [PATCH] workqueue: avoid nohz_full cores

[Tejun Heo]

Hello, Chris.

On Tue, Mar 31, 2015 at 03:25:59PM -0400, cmetcalf@ezchip.com wrote:
> From: Chris Metcalf <cmetcalf@ezchip.com>
> 
> When queuing work, we should avoid queuing it on the local cpu if
> we are using WORK_CPU_UNBOUND and the local cpu is nohz_full, since
> the workqueue will mean a later interrupt of the nohz_full process
> that presumably would prefer continuing to have 100% of the core
> without interrupts.
> 
> Likewise, remove the nohz_full cores from unbound workqueues.  If
> all the cores are nohz_full, we leave them in.

The problem with this is that workqueue currently doesn't distinguish
why work items are queued on per-cpu workqueues.  It can't tell
whether being bound to local CPU is for correctness or optimization
and thus can't break local execution guarantee for anybody.  We need
to hunt down all the ones which depend on local execution and annotate
them with explicit queue_work_on() before being able to do this.

Thanks.

-- 
tejun

Re: [PATCH] workqueue: avoid nohz_full cores

[Frederic Weisbecker]

On Tue, Mar 31, 2015 at 03:25:59PM -0400, cmetcalf@ezchip.com wrote:
> From: Chris Metcalf <cmetcalf@ezchip.com>
> 
> When queuing work, we should avoid queuing it on the local cpu if
> we are using WORK_CPU_UNBOUND and the local cpu is nohz_full, since
> the workqueue will mean a later interrupt of the nohz_full process
> that presumably would prefer continuing to have 100% of the core
> without interrupts.
> 
> Likewise, remove the nohz_full cores from unbound workqueues.  If
> all the cores are nohz_full, we leave them in.
> 
> Signed-off-by: Chris Metcalf <cmetcalf@ezchip.com>

Lai is already working on this: https://lkml.org/lkml/2015/3/12/27

All that will be needed after this patchset is to force the desired
unbound workqueue mask on boot.

Re: [PATCH] workqueue: avoid nohz_full cores

[Tejun Heo]

On Tue, Mar 31, 2015 at 10:58:41PM +0200, Frederic Weisbecker wrote:
> On Tue, Mar 31, 2015 at 03:25:59PM -0400, cmetcalf@ezchip.com wrote:
> > From: Chris Metcalf <cmetcalf@ezchip.com>
> > 
> > When queuing work, we should avoid queuing it on the local cpu if
> > we are using WORK_CPU_UNBOUND and the local cpu is nohz_full, since
> > the workqueue will mean a later interrupt of the nohz_full process
> > that presumably would prefer continuing to have 100% of the core
> > without interrupts.
> > 
> > Likewise, remove the nohz_full cores from unbound workqueues.  If
> > all the cores are nohz_full, we leave them in.
> > 
> > Signed-off-by: Chris Metcalf <cmetcalf@ezchip.com>
> 
> Lai is already working on this: https://lkml.org/lkml/2015/3/12/27
> 
> All that will be needed after this patchset is to force the desired
> unbound workqueue mask on boot.

This applies to per-cpu workqueues too, not just unbound workqueues.

Thanks.

-- 
tejun

Re: [PATCH] workqueue: avoid nohz_full cores

[Peter Zijlstra]

On Tue, Mar 31, 2015 at 03:37:45PM -0400, Tejun Heo wrote:
> Hello, Chris.
> 
> On Tue, Mar 31, 2015 at 03:25:59PM -0400, cmetcalf@ezchip.com wrote:
> > From: Chris Metcalf <cmetcalf@ezchip.com>
> > 
> > When queuing work, we should avoid queuing it on the local cpu if
> > we are using WORK_CPU_UNBOUND and the local cpu is nohz_full, since
> > the workqueue will mean a later interrupt of the nohz_full process
> > that presumably would prefer continuing to have 100% of the core
> > without interrupts.
> > 
> > Likewise, remove the nohz_full cores from unbound workqueues.  If
> > all the cores are nohz_full, we leave them in.
> 
> The problem with this is that workqueue currently doesn't distinguish
> why work items are queued on per-cpu workqueues.  It can't tell
> whether being bound to local CPU is for correctness or optimization
> and thus can't break local execution guarantee for anybody.  We need
> to hunt down all the ones which depend on local execution and annotate
> them with explicit queue_work_on() before being able to do this.

This is moot on unbound workqueues, they can run wherever so it can
never be a correctness thing, right?

Re: [PATCH] workqueue: avoid nohz_full cores

[Tejun Heo]

On Wed, Apr 01, 2015 at 09:56:07AM +0200, Peter Zijlstra wrote:
> > The problem with this is that workqueue currently doesn't distinguish
> > why work items are queued on per-cpu workqueues.  It can't tell
> > whether being bound to local CPU is for correctness or optimization
> > and thus can't break local execution guarantee for anybody.  We need
> > to hunt down all the ones which depend on local execution and annotate
> > them with explicit queue_work_on() before being able to do this.
> 
> This is moot on unbound workqueues, they can run wherever so it can
> never be a correctness thing, right?

The posted patch applies to both unbound and per-cpu workqueues.
There's a more comprehensive patchset currently being worked on by Lai
for unbound workqueues.

Thanks.

-- 
tejun