From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752477AbbERHTM (ORCPT ); Mon, 18 May 2015 03:19:12 -0400 Received: from mail-wi0-f174.google.com ([209.85.212.174]:38565 "EHLO mail-wi0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752152AbbERHTI (ORCPT ); Mon, 18 May 2015 03:19:08 -0400 From: Gabriele Mazzotta To: Peter Zijlstra 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 09:19:05 +0200 Message-ID: <1794299.10yZKyUffA@xps13> In-Reply-To: <20150518064804.GA21418@twins.programming.kicks-ass.net> References: <3301457.KOfdo1KMTP@xps13> <20150518064804.GA21418@twins.programming.kicks-ass.net> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Monday 18 May 2015 08:48:04 Peter Zijlstra wrote: > 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. Thank you for the quick reply. As I replied to Ingo's mail, which arrived just a bit earlier than yours, yes, the commit here below fixes the problem. Thanks, Gabriele > --- > commit 533445c6e53368569e50ab3fb712230c03d523f3 > Author: Omar Sandoval > 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 > #include > #include > #include > #include > #include > > 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 > Signed-off-by: Peter Zijlstra (Intel) > Cc: Borislav Petkov > Cc: H. Peter Anvin > Cc: Juri Lelli > Cc: Thomas Gleixner > 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 > > 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: