linux-kernel - Re: [PATCH] arch/x86/oprofile/op_model_amd.c: perform initialisation on a single CPU

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] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 3 Jan 2011 13:03:10 +0100
From:	Ingo Molnar <mingo@...e.hu>
To:	Robert Richter <robert.richter@....com>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"oprofile-list@...ts.sourceforge.net" 
	<oprofile-list@...ts.sourceforge.net>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Frédéric Weisbecker <fweisbec@...il.com>,
	atswartz@...il.com, "Rafael J. Wysocki" <rjw@...k.pl>,
	Dan Carpenter <error27@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>, stable@...nel.org
Subject: Re: [PATCH] arch/x86/oprofile/op_model_amd.c: perform initialisation
 on a single CPU


* Robert Richter <robert.richter@....com> wrote:

>  static void init_ibs(void)
>  {
>  	ibs_caps = get_ibs_caps();

this get_ibs_caps() call is percpu too - still it now runs with preempt off.

> +		printk(KERN_INFO "oprofile: AMD IBS detected (0x%08x)\n",
> +		       (unsigned)ibs_caps);

that cast looks ugly and unnecessary.

I fixed both in the commit below. (not tested yet)

Thanks,

	Ingo

--------------->
>From c7c25802b39c443b3745cfa973dc49a97a3491f8 Mon Sep 17 00:00:00 2001
From: Robert Richter <robert.richter@....com>
Date: Mon, 3 Jan 2011 12:15:14 +0100
Subject: [PATCH] arch/x86/oprofile/op_model_amd.c: Perform initialisation on a single CPU

Disable preemption in init_ibs(). The function only checks the
ibs capabilities and sets up pci devices (if necessary). It runs
only on one cpu but operates with the local APIC and some MSRs,
thus it is better to disable preemption.

[    7.034377] BUG: using smp_processor_id() in preemptible [00000000] code: modprobe/483
[    7.034385] caller is setup_APIC_eilvt+0x155/0x180
[    7.034389] Pid: 483, comm: modprobe Not tainted 2.6.37-rc1-20101110+ #1
[    7.034392] Call Trace:
[    7.034400]  [<ffffffff812a2b72>] debug_smp_processor_id+0xd2/0xf0
[    7.034404]  [<ffffffff8101e985>] setup_APIC_eilvt+0x155/0x180
[ ... ]

Addresses https://bugzilla.kernel.org/show_bug.cgi?id=22812

Reported-by: <atswartz@...il.com>
Signed-off-by: Robert Richter <robert.richter@....com>
Cc: oprofile-list@...ts.sourceforge.net <oprofile-list@...ts.sourceforge.net>
Cc: Peter Zijlstra <a.p.zijlstra@...llo.nl>
Cc: Frederic Weisbecker <fweisbec@...il.com>
Cc: Rafael J. Wysocki <rjw@...k.pl>
Cc: Dan Carpenter <error27@...il.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>
Cc: <stable@...nel.org>         [2.6.37.x]
LKML-Reference: <20110103111514.GM4739@...a.amd.com>
[ small cleanups ]
Signed-off-by: Ingo Molnar <mingo@...e.hu>
---
 arch/x86/oprofile/op_model_amd.c |   24 ++++++++++++++++--------
 1 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/arch/x86/oprofile/op_model_amd.c b/arch/x86/oprofile/op_model_amd.c
index a011bcc..7d90d47 100644
--- a/arch/x86/oprofile/op_model_amd.c
+++ b/arch/x86/oprofile/op_model_amd.c
@@ -630,21 +630,29 @@ static int __init_ibs_nmi(void)
 	return 0;
 }
 
-/* initialize the APIC for the IBS interrupts if available */
+/*
+ * check and reserve APIC extended interrupt LVT offset for IBS if
+ * available
+ *
+ * init_ibs() preforms implicitly cpu-local operations, so pin this
+ * thread to its current CPU
+ */
+
 static void init_ibs(void)
 {
-	ibs_caps = get_ibs_caps();
+	preempt_disable();
 
+	ibs_caps = get_ibs_caps();
 	if (!ibs_caps)
-		return;
+		goto out;
 
-	if (__init_ibs_nmi()) {
+	if (__init_ibs_nmi() < 0)
 		ibs_caps = 0;
-		return;
-	}
+	else
+		printk(KERN_INFO "oprofile: AMD IBS detected (0x%08x)\n", ibs_caps);
 
-	printk(KERN_INFO "oprofile: AMD IBS detected (0x%08x)\n",
-	       (unsigned)ibs_caps);
+out:
+	preempt_enable();
 }
 
 static int (*create_arch_files)(struct super_block *sb, struct dentry *root);
--
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/