linux-kernel - Re: [2.6.28-rc6] oprofile: "opcontrol --start" output two warnings

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: <20081125192316.26E5.KOSAKI.MOTOHIRO@jp.fujitsu.com>
Date:	Tue, 25 Nov 2008 19:29:06 +0900 (JST)
From:	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
To:	Andi Kleen <andi@...stfloor.org>
Cc:	kosaki.motohiro@...fujitsu.com,
	Robert Richter <robert.richter@....com>,
	Ingo Molnar <mingo@...e.hu>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [2.6.28-rc6] oprofile: "opcontrol --start" output two warnings

Hi

Thank you for good comment.

> KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com> writes:
> >
> > Index: b/arch/x86/oprofile/op_model_p4.c
> > ===================================================================
> > --- a/arch/x86/oprofile/op_model_p4.c	2008-11-24 06:20:04.000000000 +0900
> > +++ b/arch/x86/oprofile/op_model_p4.c	2008-11-24 06:29:09.000000000 +0900
> > @@ -399,6 +399,8 @@ static void p4_fill_in_addresses(struct 
> >  	unsigned int i;
> >  	unsigned int addr, cccraddr, stag;
> >  
> > +	get_cpu();
> 
> The whole thing seems broken anyways because the only caller of
> fill_in_addresses only assigns to CPU 0 and fill_in_addresses only
> runs on some CPU. So I guess the stagger handling could be just
> removed because it's useless.

hm,
if so, I think get_stagger() is a bit strange.
it depend on caller cpu. then if PREEMPT=Y, it return radom result.

I'm not sure about oprofile design.
but if you are right, I think p4_fill_in_addresses shoudn't use smp_processor_id().

Am I missing any point?


	static unsigned int get_stagger(void)
	{
	#ifdef CONFIG_SMP
	        int cpu = smp_processor_id();
	        return (cpu != first_cpu(per_cpu(cpu_sibling_map, cpu)));
	#endif
	        return 0;
	}



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