linux-kernel - Re: [Regression, post-2.6.34-rc1][PATCH] x86 / perf: Fix suspend to RAM on HP nx6325

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <201003212151.51383.rjw@sisk.pl>
Date:	Sun, 21 Mar 2010 21:51:51 +0100
From:	"Rafael J. Wysocki" <rjw@...k.pl>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Peter Zijlstra <peterz@...radead.org>,
	pm list <linux-pm@...ts.linux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>, Ingo Molnar <mingo@...e.hu>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [Regression, post-2.6.34-rc1][PATCH] x86 / perf: Fix suspend to RAM on HP nx6325

On Sunday 21 March 2010, Linus Torvalds wrote:
> 
> On Sat, 20 Mar 2010, Rafael J. Wysocki wrote:
> >
> > The appended patch fixes the breakage for me too.
> 
> Please don't use a 'goto' for something like this.
> 
> >  	raw_spin_lock(&amd_nb_lock);
> >  
> > +	if (!cpuhw->amd_nb)
> > +		goto unlock;
> > +
> >  	if (--cpuhw->amd_nb->refcnt == 0)
> >  		kfree(cpuhw->amd_nb);
> >  
> >  	cpuhw->amd_nb = NULL;
> >  
> > + unlock:
> >  	raw_spin_unlock(&amd_nb_lock);
> >  }
> 
> Just do
> 
> 	raw_spin_lock(&amd_nb_lock);
> 	if (cpuhw->amd_nb) {
> 		if (!--cpuhw->amd_nb->refcnt)
> 			kfree(cpuhw->amd_nb);
> 		cpuhw->amd_nb = NULL;
> 	}
> 	raw_spin_unlock(&amd_nb_lock);
> 
> instead. Much more readable.
> 
> Let's keep 'goto' for cases where we have error returns that we don't want 
> to nest, not trivial stuff like this.

OK

Rafael

---
From: Rafael J. Wysocki <rjw@...k.pl>
Subject: x86 / perf: Fix suspend to RAM on HP nx6325

Commit 3f6da3905398826d85731247e7fbcf53400c18bd
(perf: Rework and fix the arch CPU-hotplug hooks) broke suspend to
RAM on my HP nx6325 (and most likely on other AMD-based boxes too)
by allowing amd_pmu_cpu_offline() to be executed for CPUs that are
going offline as part of the suspend process.  The problem is that
cpuhw->amd_nb may be NULL already, so the function should make sure
it's not NULL before accessing the object pointed to by it.

Signed-off-by: Rafael J. Wysocki <rjw@...k.pl>
---
 arch/x86/kernel/cpu/perf_event_amd.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Index: linux-2.6/arch/x86/kernel/cpu/perf_event_amd.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/cpu/perf_event_amd.c
+++ linux-2.6/arch/x86/kernel/cpu/perf_event_amd.c
@@ -348,10 +348,12 @@ static void amd_pmu_cpu_offline(int cpu)
 
 	raw_spin_lock(&amd_nb_lock);
 
-	if (--cpuhw->amd_nb->refcnt == 0)
-		kfree(cpuhw->amd_nb);
+	if (cpuhw->amd_nb) {
+		if (--cpuhw->amd_nb->refcnt == 0)
+			kfree(cpuhw->amd_nb);
 
-	cpuhw->amd_nb = NULL;
+		cpuhw->amd_nb = NULL;
+	}
 
 	raw_spin_unlock(&amd_nb_lock);
 }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/