All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: torvalds@linux-foundation.org, umgwanakikbuti@gmail.com,
	mhocko@kernel.org, jslaby@suse.cz, tglx@linutronix.de,
	pmladek@suse.com, jack@suse.cz, ben@decadent.org.uk,
	sasha.levin@oracle.com, shli@fb.com, daniel.bilik@neosystem.cz,
	gregkh@linuxfoundation.org
Cc: linux-kernel@vger.kernel.org, kernel-team@fb.com,
	Tejun Heo <tj@kernel.org>,
	stable@vger.kernel.org,
	Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Subject: [PATCH 1/3] Revert "workqueue: make sure delayed work run in local cpu"
Date: Tue,  9 Feb 2016 18:14:48 -0500	[thread overview]
Message-ID: <1455059690-18765-2-git-send-email-tj@kernel.org> (raw)
In-Reply-To: <1455059690-18765-1-git-send-email-tj@kernel.org>

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

  reply	other threads:[~2016-02-09 23:16 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-09 23:14 [PATCHSET] workqueue: break local execution guarantee of unbound work items Tejun Heo
2016-02-09 23:14 ` Tejun Heo [this message]
2016-02-15 13:14   ` [PATCH 1/3] Revert "workqueue: make sure delayed work run in local cpu" Michal Hocko
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 ` [PATCH 3/3] workqueue: implement "workqueue.debug_force_rr_cpu" debug feature 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
2016-02-10 15:57   ` 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=1455059690-18765-2-git-send-email-tj@kernel.org \
    --to=tj@kernel.org \
    --cc=ben@decadent.org.uk \
    --cc=daniel.bilik@neosystem.cz \
    --cc=gregkh@linuxfoundation.org \
    --cc=hmh@hmh.eng.br \
    --cc=jack@suse.cz \
    --cc=jslaby@suse.cz \
    --cc=kernel-team@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhocko@kernel.org \
    --cc=pmladek@suse.com \
    --cc=sasha.levin@oracle.com \
    --cc=shli@fb.com \
    --cc=stable@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=umgwanakikbuti@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.