linux-kernel - Re: [GIT PULL] oprofile fixes for v2.6.37

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]

Message-ID: <20101230173847.GA2734@elte.hu>
Date:	Thu, 30 Dec 2010 18:38:47 +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>
Subject: Re: [GIT PULL] oprofile fixes for v2.6.37


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

> On 29.12.10 11:37:43, Ingo Molnar wrote:
> 
> > Hm, i'm not sure this fix is correct:
> > 
> >  static int op_amd_init(struct oprofile_operations *ops)
> >  {
> > +       /*
> > +        * init_ibs() preforms implictly cpu-local operations, so pin this
> > +        * thread to its current CPU
> > +        */
> > +       preempt_disable();
> >         init_ibs();
> > +       preempt_enable();
> > 
> > If init_ibs() is indeed CPU local, then it needs to be called on all CPUs. Does 
> > that happen and if not why not? AFAICS it's only called on one CPU.
> 
> It is correct to run init_ibs() only on one cpu.  It 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.

Ok, but in that case the prempt_disable()/enable() should be put into init_ibs(), 
not be open-coded at the caller like that.

The comment about its cpu-localness could move to init_ibs() as well.

Thanks,

	Ingo
--
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/