* [PATCH 1/3] Revert "workqueue: make sure delayed work run in local cpu"
2016-02-09 23:14 [PATCHSET] workqueue: break local execution guarantee of unbound work items Tejun Heo
@ 2016-02-09 23:14 ` Tejun Heo
2016-02-15 13:14 ` Michal Hocko
2016-02-09 23:14 ` [PATCH 2/3] workqueue: schedule WORK_CPU_UNBOUND work on wq_unbound_cpumask CPUs Tejun Heo
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Tejun Heo @ 2016-02-09 23:14 UTC (permalink / raw)
To: torvalds, umgwanakikbuti, mhocko, jslaby, tglx, pmladek, jack,
ben, sasha.levin, shli, daniel.bilik, gregkh
Cc: linux-kernel, kernel-team, Tejun Heo, stable,
Henrique de Moraes Holschuh
This reverts commit 874bbfe600a660cba9c776b3957b1ce393151b76.
Workqueue used to implicity guarantee that work items queued without
explicit CPU specified are put on the local CPU. Recent changes in
timer broke the guarantee and led to vmstat breakage which was fixed
by 176bed1de5bf ("vmstat: explicitly schedule per-cpu work on the CPU
we need it to run on").
vmstat is the most likely to expose the issue and it's quite possible
that there are other similar problems which are a lot more difficult
to trigger. As a preventive measure, 874bbfe600a6 ("workqueue: make
sure delayed work run in local cpu") was applied to restore the local
CPU guarnatee. Unfortunately, the change exposed a bug in timer code
which got fixed by 22b886dd1018 ("timers: Use proper base migration in
add_timer_on()"). Due to code restructuring, the commit couldn't be
backported beyond certain point and stable kernels which only had
874bbfe600a6 started crashing.
The local CPU guarantee was accidental more than anything else and we
want to get rid of it anyway. As, with the vmstat case fixed,
874bbfe600a6 is causing more problems than it's fixing, it has been
decided to take the chance and officially break the guarantee by
reverting the commit. A debug feature will be added to force foreign
CPU assignment to expose cases relying on the guarantee and fixes for
the individual cases will be backported to stable as necessary.
Signed-off-by: Tejun Heo <tj@kernel.org>
Fixes: 874bbfe600a6 ("workqueue: make sure delayed work run in local cpu")
Link: http://lkml.kernel.org/g/20160120211926.GJ10810@quack.suse.cz
Cc: stable@vger.kernel.org
Cc: Mike Galbraith <umgwanakikbuti@gmail.com>
Cc: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Cc: Daniel Bilik <daniel.bilik@neosystem.cz>
Cc: Jan Kara <jack@suse.cz>
Cc: Shaohua Li <shli@fb.com>
Cc: Sasha Levin <sasha.levin@oracle.com>
Cc: Ben Hutchings <ben@decadent.org.uk>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Daniel Bilik <daniel.bilik@neosystem.cz>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: Michal Hocko <mhocko@kernel.org>
---
kernel/workqueue.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index dc7faad..5e63d3b 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -1464,13 +1464,13 @@ static void __queue_delayed_work(int cpu, struct workqueue_struct *wq,
timer_stats_timer_set_start_info(&dwork->timer);
dwork->wq = wq;
- /* timer isn't guaranteed to run in this cpu, record earlier */
- if (cpu == WORK_CPU_UNBOUND)
- cpu = raw_smp_processor_id();
dwork->cpu = cpu;
timer->expires = jiffies + delay;
- add_timer_on(timer, cpu);
+ if (unlikely(cpu != WORK_CPU_UNBOUND))
+ add_timer_on(timer, cpu);
+ else
+ add_timer(timer);
}
/**
--
2.5.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 1/3] Revert "workqueue: make sure delayed work run in local cpu"
2016-02-09 23:14 ` [PATCH 1/3] Revert "workqueue: make sure delayed work run in local cpu" Tejun Heo
@ 2016-02-15 13:14 ` Michal Hocko
0 siblings, 0 replies; 9+ messages in thread
From: Michal Hocko @ 2016-02-15 13:14 UTC (permalink / raw)
To: Tejun Heo
Cc: torvalds, umgwanakikbuti, jslaby, tglx, pmladek, jack, ben,
sasha.levin, shli, daniel.bilik, gregkh, linux-kernel,
kernel-team, stable, Henrique de Moraes Holschuh
On Tue 09-02-16 18:14:48, Tejun Heo wrote:
> This reverts commit 874bbfe600a660cba9c776b3957b1ce393151b76.
>
> Workqueue used to implicity guarantee that work items queued without
> explicit CPU specified are put on the local CPU. Recent changes in
> timer broke the guarantee and led to vmstat breakage which was fixed
> by 176bed1de5bf ("vmstat: explicitly schedule per-cpu work on the CPU
> we need it to run on").
>
> vmstat is the most likely to expose the issue and it's quite possible
> that there are other similar problems which are a lot more difficult
> to trigger. As a preventive measure, 874bbfe600a6 ("workqueue: make
> sure delayed work run in local cpu") was applied to restore the local
> CPU guarnatee. Unfortunately, the change exposed a bug in timer code
> which got fixed by 22b886dd1018 ("timers: Use proper base migration in
> add_timer_on()"). Due to code restructuring, the commit couldn't be
> backported beyond certain point and stable kernels which only had
> 874bbfe600a6 started crashing.
>
> The local CPU guarantee was accidental more than anything else and we
> want to get rid of it anyway. As, with the vmstat case fixed,
I would find it helpful to mention 176bed1de5bf ("vmstat: explicitly
schedule per-cpu work on the CPU we need it to run on") as the vmstat
fix.
> 874bbfe600a6 is causing more problems than it's fixing, it has been
> decided to take the chance and officially break the guarantee by
> reverting the commit. A debug feature will be added to force foreign
> CPU assignment to expose cases relying on the guarantee and fixes for
> the individual cases will be backported to stable as necessary.
>
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Fixes: 874bbfe600a6 ("workqueue: make sure delayed work run in local cpu")
> Link: http://lkml.kernel.org/g/20160120211926.GJ10810@quack.suse.cz
> Cc: stable@vger.kernel.org
> Cc: Mike Galbraith <umgwanakikbuti@gmail.com>
> Cc: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
> Cc: Daniel Bilik <daniel.bilik@neosystem.cz>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Shaohua Li <shli@fb.com>
> Cc: Sasha Levin <sasha.levin@oracle.com>
> Cc: Ben Hutchings <ben@decadent.org.uk>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Daniel Bilik <daniel.bilik@neosystem.cz>
> Cc: Jiri Slaby <jslaby@suse.cz>
> Cc: Michal Hocko <mhocko@kernel.org>
Acked-by: Michal Hocko <mhocko@suse.com>
Thanks!
> ---
> kernel/workqueue.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index dc7faad..5e63d3b 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -1464,13 +1464,13 @@ static void __queue_delayed_work(int cpu, struct workqueue_struct *wq,
> timer_stats_timer_set_start_info(&dwork->timer);
>
> dwork->wq = wq;
> - /* timer isn't guaranteed to run in this cpu, record earlier */
> - if (cpu == WORK_CPU_UNBOUND)
> - cpu = raw_smp_processor_id();
> dwork->cpu = cpu;
> timer->expires = jiffies + delay;
>
> - add_timer_on(timer, cpu);
> + if (unlikely(cpu != WORK_CPU_UNBOUND))
> + add_timer_on(timer, cpu);
> + else
> + add_timer(timer);
> }
>
> /**
> --
> 2.5.0
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] workqueue: schedule WORK_CPU_UNBOUND work on wq_unbound_cpumask CPUs
2016-02-09 23:14 [PATCHSET] workqueue: break local execution guarantee of unbound work items Tejun Heo
2016-02-09 23:14 ` [PATCH 1/3] Revert "workqueue: make sure delayed work run in local cpu" Tejun Heo
@ 2016-02-09 23:14 ` Tejun Heo
2016-02-09 23:14 ` [PATCH 3/3] workqueue: implement "workqueue.debug_force_rr_cpu" debug feature Tejun Heo
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Tejun Heo @ 2016-02-09 23:14 UTC (permalink / raw)
To: torvalds, umgwanakikbuti, mhocko, jslaby, tglx, pmladek, jack,
ben, sasha.levin, shli, daniel.bilik, gregkh
Cc: linux-kernel, kernel-team, Tejun Heo
From: Mike Galbraith <umgwanakikbuti@gmail.com>
WORK_CPU_UNBOUND work items queued to a bound workqueue always run
locally. This is a good thing normally, but not when the user has
asked us to keep unbound work away from certain CPUs. Round robin
these to wq_unbound_cpumask CPUs instead, as perturbation avoidance
trumps performance.
tj: Cosmetic and comment changes. WARN_ON_ONCE() dropped from empty
(wq_unbound_cpumask AND cpu_online_mask). If we want that, it
should be done when config changes.
Signed-off-by: Mike Galbraith <umgwanakikbuti@gmail.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
---
kernel/workqueue.c | 34 ++++++++++++++++++++++++++++++++--
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 5e63d3b..0547746 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -301,7 +301,11 @@ static DEFINE_SPINLOCK(wq_mayday_lock); /* protects wq->maydays list */
static LIST_HEAD(workqueues); /* PR: list of all workqueues */
static bool workqueue_freezing; /* PL: have wqs started freezing? */
-static cpumask_var_t wq_unbound_cpumask; /* PL: low level cpumask for all unbound wqs */
+/* PL: allowable cpus for unbound wqs and work items */
+static cpumask_var_t wq_unbound_cpumask;
+
+/* CPU where unbound work was last round robin scheduled from this CPU */
+static DEFINE_PER_CPU(int, wq_rr_cpu_last);
/* the per-cpu worker pools */
static DEFINE_PER_CPU_SHARED_ALIGNED(struct worker_pool [NR_STD_WORKER_POOLS],
@@ -1298,6 +1302,32 @@ static bool is_chained_work(struct workqueue_struct *wq)
return worker && worker->current_pwq->wq == wq;
}
+/*
+ * When queueing an unbound work item to a wq, prefer local CPU if allowed
+ * by wq_unbound_cpumask. Otherwise, round robin among the allowed ones to
+ * avoid perturbing sensitive tasks.
+ */
+static int wq_select_unbound_cpu(int cpu)
+{
+ int new_cpu;
+
+ if (cpumask_test_cpu(cpu, wq_unbound_cpumask))
+ return cpu;
+ if (cpumask_empty(wq_unbound_cpumask))
+ return cpu;
+
+ new_cpu = __this_cpu_read(wq_rr_cpu_last);
+ new_cpu = cpumask_next_and(new_cpu, wq_unbound_cpumask, cpu_online_mask);
+ if (unlikely(new_cpu >= nr_cpu_ids)) {
+ new_cpu = cpumask_first_and(wq_unbound_cpumask, cpu_online_mask);
+ if (unlikely(new_cpu >= nr_cpu_ids))
+ return cpu;
+ }
+ __this_cpu_write(wq_rr_cpu_last, new_cpu);
+
+ return new_cpu;
+}
+
static void __queue_work(int cpu, struct workqueue_struct *wq,
struct work_struct *work)
{
@@ -1323,7 +1353,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 = wq_select_unbound_cpu(raw_smp_processor_id());
/* pwq which will be used unless @work is executing elsewhere */
if (!(wq->flags & WQ_UNBOUND))
--
2.5.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/3] workqueue: implement "workqueue.debug_force_rr_cpu" debug feature
2016-02-09 23:14 [PATCHSET] workqueue: break local execution guarantee of unbound work items Tejun Heo
2016-02-09 23:14 ` [PATCH 1/3] Revert "workqueue: make sure delayed work run in local cpu" Tejun Heo
2016-02-09 23:14 ` [PATCH 2/3] workqueue: schedule WORK_CPU_UNBOUND work on wq_unbound_cpumask CPUs Tejun Heo
@ 2016-02-09 23:14 ` Tejun Heo
2016-02-15 13:18 ` Michal Hocko
2016-02-10 0:53 ` [PATCHSET] workqueue: break local execution guarantee of unbound work items Linus Torvalds
2016-02-10 8:01 ` Jiri Slaby
4 siblings, 1 reply; 9+ messages in thread
From: Tejun Heo @ 2016-02-09 23:14 UTC (permalink / raw)
To: torvalds, umgwanakikbuti, mhocko, jslaby, tglx, pmladek, jack,
ben, sasha.levin, shli, daniel.bilik, gregkh
Cc: linux-kernel, kernel-team, Tejun Heo
Workqueue used to guarantee local execution for work items queued
without explicit target CPU. The guarantee is gone now which can
break some usages in subtle ways. To flush out those cases, this
patch implements a debug feature which forces round-robin CPU
selection for all such work items.
The debug feature defaults to off and can be enabled with a kernel
parameter. The default can be flipped with a debug config option.
If you hit this commit during bisection, please refer to 041bd12e272c
("Revert "workqueue: make sure delayed work run in local cpu"") for
more information and ping me.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
Documentation/kernel-parameters.txt | 11 +++++++++++
kernel/workqueue.c | 23 +++++++++++++++++++++--
lib/Kconfig.debug | 15 +++++++++++++++
3 files changed, 47 insertions(+), 2 deletions(-)
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 87d40a7..cda2ead 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -4230,6 +4230,17 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
The default value of this parameter is determined by
the config option CONFIG_WQ_POWER_EFFICIENT_DEFAULT.
+ workqueue.debug_force_rr_cpu
+ Workqueue used to implicitly guarantee that work
+ items queued without explicit CPU specified are put
+ on the local CPU. This guarantee is no longer true
+ and while local CPU is still preferred work items
+ may be put on foreign CPUs. This debug option
+ forces round-robin CPU selection to flush out
+ usages which depend on the now broken guarantee.
+ When enabled, memory and cache locality will be
+ impacted.
+
x2apic_phys [X86-64,APIC] Use x2apic physical mode instead of
default x2apic cluster mode on platforms
supporting x2apic.
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 0547746..51d77e7 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -307,6 +307,18 @@ static cpumask_var_t wq_unbound_cpumask;
/* CPU where unbound work was last round robin scheduled from this CPU */
static DEFINE_PER_CPU(int, wq_rr_cpu_last);
+/*
+ * Local execution of unbound work items is no longer guaranteed. The
+ * following always forces round-robin CPU selection on unbound work items
+ * to uncover usages which depend on it.
+ */
+#ifdef CONFIG_DEBUG_WQ_FORCE_RR_CPU
+static bool wq_debug_force_rr_cpu = true;
+#else
+static bool wq_debug_force_rr_cpu = false;
+#endif
+module_param_named(debug_force_rr_cpu, wq_debug_force_rr_cpu, bool, 0644);
+
/* the per-cpu worker pools */
static DEFINE_PER_CPU_SHARED_ALIGNED(struct worker_pool [NR_STD_WORKER_POOLS],
cpu_worker_pools);
@@ -1309,10 +1321,17 @@ static bool is_chained_work(struct workqueue_struct *wq)
*/
static int wq_select_unbound_cpu(int cpu)
{
+ static bool printed_dbg_warning;
int new_cpu;
- if (cpumask_test_cpu(cpu, wq_unbound_cpumask))
- return cpu;
+ if (likely(!wq_debug_force_rr_cpu)) {
+ if (cpumask_test_cpu(cpu, wq_unbound_cpumask))
+ return cpu;
+ } else if (!printed_dbg_warning) {
+ pr_warn("workqueue: round-robin CPU selection forced, expect performance impact\n");
+ printed_dbg_warning = true;
+ }
+
if (cpumask_empty(wq_unbound_cpumask))
return cpu;
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index ecb9e75..8bfd1ac 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1400,6 +1400,21 @@ config RCU_EQS_DEBUG
endmenu # "RCU Debugging"
+config DEBUG_WQ_FORCE_RR_CPU
+ bool "Force round-robin CPU selection for unbound work items"
+ depends on DEBUG_KERNEL
+ default n
+ help
+ Workqueue used to implicitly guarantee that work items queued
+ without explicit CPU specified are put on the local CPU. This
+ guarantee is no longer true and while local CPU is still
+ preferred work items may be put on foreign CPUs. Kernel
+ parameter "workqueue.debug_force_rr_cpu" is added to force
+ round-robin CPU selection to flush out usages which depend on the
+ now broken guarantee. This config option enables the debug
+ feature by default. When enabled, memory and cache locality will
+ be impacted.
+
config DEBUG_BLOCK_EXT_DEVT
bool "Force extended block device numbers and spread them"
depends on DEBUG_KERNEL
--
2.5.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 3/3] workqueue: implement "workqueue.debug_force_rr_cpu" debug feature
2016-02-09 23:14 ` [PATCH 3/3] workqueue: implement "workqueue.debug_force_rr_cpu" debug feature Tejun Heo
@ 2016-02-15 13:18 ` Michal Hocko
0 siblings, 0 replies; 9+ messages in thread
From: Michal Hocko @ 2016-02-15 13:18 UTC (permalink / raw)
To: Tejun Heo
Cc: torvalds, umgwanakikbuti, jslaby, tglx, pmladek, jack, ben,
sasha.levin, shli, daniel.bilik, gregkh, linux-kernel,
kernel-team
On Tue 09-02-16 18:14:50, Tejun Heo wrote:
> Workqueue used to guarantee local execution for work items queued
> without explicit target CPU. The guarantee is gone now which can
> break some usages in subtle ways. To flush out those cases, this
> patch implements a debug feature which forces round-robin CPU
> selection for all such work items.
>
> The debug feature defaults to off and can be enabled with a kernel
> parameter. The default can be flipped with a debug config option.
Makes sense to me
>
> If you hit this commit during bisection, please refer to 041bd12e272c
> ("Revert "workqueue: make sure delayed work run in local cpu"") for
> more information and ping me.
>
> Signed-off-by: Tejun Heo <tj@kernel.org>
Acked-by: Michal Hocko <mhocko@suse.com>
Thanks!
> ---
> Documentation/kernel-parameters.txt | 11 +++++++++++
> kernel/workqueue.c | 23 +++++++++++++++++++++--
> lib/Kconfig.debug | 15 +++++++++++++++
> 3 files changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 87d40a7..cda2ead 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -4230,6 +4230,17 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
> The default value of this parameter is determined by
> the config option CONFIG_WQ_POWER_EFFICIENT_DEFAULT.
>
> + workqueue.debug_force_rr_cpu
> + Workqueue used to implicitly guarantee that work
> + items queued without explicit CPU specified are put
> + on the local CPU. This guarantee is no longer true
> + and while local CPU is still preferred work items
> + may be put on foreign CPUs. This debug option
> + forces round-robin CPU selection to flush out
> + usages which depend on the now broken guarantee.
> + When enabled, memory and cache locality will be
> + impacted.
> +
> x2apic_phys [X86-64,APIC] Use x2apic physical mode instead of
> default x2apic cluster mode on platforms
> supporting x2apic.
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index 0547746..51d77e7 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -307,6 +307,18 @@ static cpumask_var_t wq_unbound_cpumask;
> /* CPU where unbound work was last round robin scheduled from this CPU */
> static DEFINE_PER_CPU(int, wq_rr_cpu_last);
>
> +/*
> + * Local execution of unbound work items is no longer guaranteed. The
> + * following always forces round-robin CPU selection on unbound work items
> + * to uncover usages which depend on it.
> + */
> +#ifdef CONFIG_DEBUG_WQ_FORCE_RR_CPU
> +static bool wq_debug_force_rr_cpu = true;
> +#else
> +static bool wq_debug_force_rr_cpu = false;
> +#endif
> +module_param_named(debug_force_rr_cpu, wq_debug_force_rr_cpu, bool, 0644);
> +
> /* the per-cpu worker pools */
> static DEFINE_PER_CPU_SHARED_ALIGNED(struct worker_pool [NR_STD_WORKER_POOLS],
> cpu_worker_pools);
> @@ -1309,10 +1321,17 @@ static bool is_chained_work(struct workqueue_struct *wq)
> */
> static int wq_select_unbound_cpu(int cpu)
> {
> + static bool printed_dbg_warning;
> int new_cpu;
>
> - if (cpumask_test_cpu(cpu, wq_unbound_cpumask))
> - return cpu;
> + if (likely(!wq_debug_force_rr_cpu)) {
> + if (cpumask_test_cpu(cpu, wq_unbound_cpumask))
> + return cpu;
> + } else if (!printed_dbg_warning) {
> + pr_warn("workqueue: round-robin CPU selection forced, expect performance impact\n");
> + printed_dbg_warning = true;
> + }
> +
> if (cpumask_empty(wq_unbound_cpumask))
> return cpu;
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index ecb9e75..8bfd1ac 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -1400,6 +1400,21 @@ config RCU_EQS_DEBUG
>
> endmenu # "RCU Debugging"
>
> +config DEBUG_WQ_FORCE_RR_CPU
> + bool "Force round-robin CPU selection for unbound work items"
> + depends on DEBUG_KERNEL
> + default n
> + help
> + Workqueue used to implicitly guarantee that work items queued
> + without explicit CPU specified are put on the local CPU. This
> + guarantee is no longer true and while local CPU is still
> + preferred work items may be put on foreign CPUs. Kernel
> + parameter "workqueue.debug_force_rr_cpu" is added to force
> + round-robin CPU selection to flush out usages which depend on the
> + now broken guarantee. This config option enables the debug
> + feature by default. When enabled, memory and cache locality will
> + be impacted.
> +
> config DEBUG_BLOCK_EXT_DEVT
> bool "Force extended block device numbers and spread them"
> depends on DEBUG_KERNEL
> --
> 2.5.0
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCHSET] workqueue: break local execution guarantee of unbound work items
2016-02-09 23:14 [PATCHSET] workqueue: break local execution guarantee of unbound work items Tejun Heo
` (2 preceding siblings ...)
2016-02-09 23:14 ` [PATCH 3/3] workqueue: implement "workqueue.debug_force_rr_cpu" debug feature Tejun Heo
@ 2016-02-10 0:53 ` Linus Torvalds
2016-02-10 8:01 ` Jiri Slaby
4 siblings, 0 replies; 9+ messages in thread
From: Linus Torvalds @ 2016-02-10 0:53 UTC (permalink / raw)
To: Tejun Heo
Cc: Mike Galbraith, Michal Hocko, Jiri Slaby, Thomas Gleixner,
Petr Mladek, Jan Kara, Ben Hutchings, Sasha Levin, Shaohua Li,
Daniel Bilik, Greg Kroah-Hartman, Linux Kernel Mailing List,
kernel-team
On Tue, Feb 9, 2016 at 3:14 PM, Tejun Heo <tj@kernel.org> wrote:
>
> This patchset reverts 874bbfe600a6 ("workqueue: make sure delayed work
> run in local cpu"), expands wq_unbound_cpu_mask so that it also
> applies to unbound work items queued on percpu workqueues, and
> implements a debug feature which forces wq_unbound_cpu_mask based
> round-robin selection to flush out usages which depend on the local
> execution guarantee.
Looks all very reasonable to me.
Linus
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCHSET] workqueue: break local execution guarantee of unbound work items
2016-02-09 23:14 [PATCHSET] workqueue: break local execution guarantee of unbound work items Tejun Heo
` (3 preceding siblings ...)
2016-02-10 0:53 ` [PATCHSET] workqueue: break local execution guarantee of unbound work items Linus Torvalds
@ 2016-02-10 8:01 ` Jiri Slaby
2016-02-10 15:57 ` Tejun Heo
4 siblings, 1 reply; 9+ messages in thread
From: Jiri Slaby @ 2016-02-10 8:01 UTC (permalink / raw)
To: Tejun Heo, torvalds, umgwanakikbuti, mhocko, tglx, pmladek, jack,
ben, sasha.levin, shli, daniel.bilik, gregkh
Cc: linux-kernel, kernel-team
On 02/10/2016, 12:14 AM, Tejun Heo wrote:
> Hello,
>
> Workqueue used to implicitly guarantee local execution of unbound work
> items. Recent timer updates broke that for delayed work items and the
> attempt to restore it ended up causing more harm than good. It has
> been decided to take the chance and officially break it.
>
> This patchset reverts 874bbfe600a6 ("workqueue: make sure delayed work
> run in local cpu"), expands wq_unbound_cpu_mask so that it also
> applies to unbound work items queued on percpu workqueues, and
> implements a debug feature which forces wq_unbound_cpu_mask based
> round-robin selection to flush out usages which depend on the local
> execution guarantee.
>
> I'll push the patchset through wq/for-4.5-fixes soon.
>
> The patchset contains the following three patches.
>
> 0001-Revert-workqueue-make-sure-delayed-work-run-in-local.patch
> 0002-workqueue-schedule-WORK_CPU_UNBOUND-work-on-wq_unbou.patch
> 0003-workqueue-implement-workqueue.debug_force_rr_cpu-deb.patch
Thanks all for sorting the issue out. Now it remains to decide what
should go to stable. Given only 0001 is marked as "Fixes", is that enough?
And what about the vmstat fix:
commit 176bed1de5bf977938cad26551969eca8f0883b1
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date: Thu Oct 15 13:01:50 2015 -0700
vmstat: explicitly schedule per-cpu work on the CPU we need it to run on
? It should fix better than what 0001 is reverting AFAIU, right?
thanks,
--
js
suse labs
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCHSET] workqueue: break local execution guarantee of unbound work items
2016-02-10 8:01 ` Jiri Slaby
@ 2016-02-10 15:57 ` Tejun Heo
0 siblings, 0 replies; 9+ messages in thread
From: Tejun Heo @ 2016-02-10 15:57 UTC (permalink / raw)
To: Jiri Slaby
Cc: torvalds, umgwanakikbuti, mhocko, tglx, pmladek, jack, ben,
sasha.levin, shli, daniel.bilik, gregkh, linux-kernel,
kernel-team
Hello, Jiri.
On Wed, Feb 10, 2016 at 09:01:38AM +0100, Jiri Slaby wrote:
> Thanks all for sorting the issue out. Now it remains to decide what
> should go to stable. Given only 0001 is marked as "Fixes", is that enough?
I think so. The other two are primarily for debugging anyway.
> And what about the vmstat fix:
> commit 176bed1de5bf977938cad26551969eca8f0883b1
> Author: Linus Torvalds <torvalds@linux-foundation.org>
> Date: Thu Oct 15 13:01:50 2015 -0700
>
> vmstat: explicitly schedule per-cpu work on the CPU we need it to run on
>
> ? It should fix better than what 0001 is reverting AFAIU, right?
Yes, definitely. Didn't know that one wasn't tagged for stable.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 9+ messages in thread