All of lore.kernel.org
 help / color / mirror / Atom feed
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:

  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.