From: Peter Zijlstra <peterz@infradead.org>
To: Gabriele Mazzotta <gabriele.mzt@gmail.com>
Cc: juri.lelli@arm.com, mingo@redhat.com,
linux-kernel@vger.kernel.org, len.brown@intel.com,
andrey.semin@intel.com
Subject: Re: Regression: turbostat stops working after suspend/resume cycle\
Date: Mon, 18 May 2015 08:48:04 +0200 [thread overview]
Message-ID: <20150518064804.GA21418@twins.programming.kicks-ass.net> (raw)
In-Reply-To: <3301457.KOfdo1KMTP@xps13>
On Sun, May 17, 2015 at 05:48:44PM +0200, Gabriele Mazzotta wrote:
> Hi,
>
> I've recently noticed that if I suspend and resume my laptop, I can no
> longer execute turbostat. This is what I get when I try to start it:
> # turbostat
> Could not migrate to CPU 1
> turbostat: re-initialized with num_cpus 4
> Could not migrate to CPU 1
>
> Since everything works as expected with v4.0, I ran a bisection and
> found that commit 3c18d447b3b36a8d ("sched/core: Check for available
> DL bandwidth in cpuset_cpu_inactive()") is the cause of the regression.
>
> I don't know if there's something else affected by that change, but
> I can consistently reproduce the bug with turbostat.
This should be fixed by the below commit which is already in Linus'
tree.
---
commit 533445c6e53368569e50ab3fb712230c03d523f3
Author: Omar Sandoval <osandov@osandov.com>
Date: Mon May 4 03:09:36 2015 -0700
sched/core: Fix regression in cpuset_cpu_inactive() for suspend
Commit 3c18d447b3b3 ("sched/core: Check for available DL bandwidth in
cpuset_cpu_inactive()"), a SCHED_DEADLINE bugfix, had a logic error that
caused a regression in setting a CPU inactive during suspend. I ran into
this when a program was failing pthread_setaffinity_np() with EINVAL after
a suspend+wake up.
A simple reproducer:
$ ./a.out
sched_setaffinity: Success
$ systemctl suspend
$ ./a.out
sched_setaffinity: Invalid argument
... where ./a.out is:
#define _GNU_SOURCE
#include <errno.h>
#include <sched.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
int main(void)
{
long num_cores;
cpu_set_t cpu_set;
int ret;
num_cores = sysconf(_SC_NPROCESSORS_ONLN);
CPU_ZERO(&cpu_set);
CPU_SET(num_cores - 1, &cpu_set);
errno = 0;
ret = sched_setaffinity(getpid(), sizeof(cpu_set), &cpu_set);
perror("sched_setaffinity");
return ret ? EXIT_FAILURE : EXIT_SUCCESS;
}
The mistake is that suspend is handled in the action ==
CPU_DOWN_PREPARE_FROZEN case of the switch statement in
cpuset_cpu_inactive().
However, the commit in question masked out CPU_TASKS_FROZEN
from the action, making this case dead.
The fix is straightforward.
Signed-off-by: Omar Sandoval <osandov@osandov.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Juri Lelli <juri.lelli@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Fixes: 3c18d447b3b3 ("sched/core: Check for available DL bandwidth in cpuset_cpu_inactive()")
Link: http://lkml.kernel.org/r/1cb5ecb3d6543c38cce5790387f336f54ec8e2bc.1430733960.git.osandov@osandov.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 34db9bf892a3..57bd333bc4ab 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6999,27 +6999,23 @@ static int cpuset_cpu_inactive(struct notifier_block *nfb, unsigned long action,
unsigned long flags;
long cpu = (long)hcpu;
struct dl_bw *dl_b;
+ bool overflow;
+ int cpus;
- switch (action & ~CPU_TASKS_FROZEN) {
+ switch (action) {
case CPU_DOWN_PREPARE:
- /* explicitly allow suspend */
- if (!(action & CPU_TASKS_FROZEN)) {
- bool overflow;
- int cpus;
-
- rcu_read_lock_sched();
- dl_b = dl_bw_of(cpu);
+ rcu_read_lock_sched();
+ dl_b = dl_bw_of(cpu);
- raw_spin_lock_irqsave(&dl_b->lock, flags);
- cpus = dl_bw_cpus(cpu);
- overflow = __dl_overflow(dl_b, cpus, 0, 0);
- raw_spin_unlock_irqrestore(&dl_b->lock, flags);
+ raw_spin_lock_irqsave(&dl_b->lock, flags);
+ cpus = dl_bw_cpus(cpu);
+ overflow = __dl_overflow(dl_b, cpus, 0, 0);
+ raw_spin_unlock_irqrestore(&dl_b->lock, flags);
- rcu_read_unlock_sched();
+ rcu_read_unlock_sched();
- if (overflow)
- return notifier_from_errno(-EBUSY);
- }
+ if (overflow)
+ return notifier_from_errno(-EBUSY);
cpuset_update_active_cpus(false);
break;
case CPU_DOWN_PREPARE_FROZEN:
next prev parent reply other threads:[~2015-05-18 6:48 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-17 15:48 Regression: turbostat stops working after suspend/resume cycle Gabriele Mazzotta
2015-05-18 6:45 ` Ingo Molnar
2015-05-18 7:12 ` Gabriele Mazzotta
2015-05-18 6:48 ` Peter Zijlstra [this message]
2015-05-18 7:19 ` Regression: turbostat stops working after suspend/resume cycle\ Gabriele Mazzotta
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=20150518064804.GA21418@twins.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=andrey.semin@intel.com \
--cc=gabriele.mzt@gmail.com \
--cc=juri.lelli@arm.com \
--cc=len.brown@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.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.