From mboxrd@z Thu Jan 1 00:00:00 1970 From: Robert Richter Date: Wed, 06 Oct 2010 19:22:35 +0000 Subject: Re: [PATCH 4/7] ARM: oprofile: Move non-ARM code into separate Message-Id: <20101006192235.GA13563@erda.amd.com> List-Id: References: <74092b0d162aafc21e408033cec6146e227363b5.1286222593.git.matt@console-pimps.org> <20101006133309.GP13563@erda.amd.com> <20101006144921.GQ13563@erda.amd.com> <20101006145311.GA3754@linux-sh.org> <20101006145903.GR13563@erda.amd.com> <20101006184407.GY13563@erda.amd.com> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable To: Grant Likely Cc: Paul Mundt , Matt Fleming , Will Deacon , Russell King , "linux-arm-kernel@lists.infradead.org" , "linux-sh@vger.kernel.org" , Peter Zijlstra , Ingo Molnar , Frederic Weisbecker , Arnaldo Carvalho de Melo , "linux-arch@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Deng-Cheng Zhu On 06.10.10 14:58:06, Grant Likely wrote: > On Wed, Oct 6, 2010 at 12:44 PM, Robert Richter = wrote: > > On 06.10.10 14:23:17, Grant Likely wrote: > >> >> diff --git a/drivers/oprofile/oprof.c b/drivers/oprofile/oprof.c > >> >> index b336cd9..76d97a5 100644 > >> >> --- a/drivers/oprofile/oprof.c > >> >> +++ b/drivers/oprofile/oprof.c > >> >> @@ -252,6 +252,8 @@ static int __init oprofile_init(void) > >> >> =A0{ > >> >> =A0 =A0 =A0 int err; > >> >> > >> >> + =A0 =A0 memset(&oprofile_ops, 0, sizeof(struct oprofile_operation= s)); > >> >> + > >> > > >> > Yes, this looks much better. Thanks Paul. > >> > >> oprofile_ops is allocated in bss and so is already zeroed. =A0No need = to > >> do it explicitly. > > > > Before oprofile_arch_init() it isn't necessarily needed, but we should > > definitely add it before oprofile_timer_init(). For code readability I > > would rather tend to let the first in too. >=20 > ? Sorry, I don't understand what you're trying to say. Because > oprofile_ops is statically allocated in bss; it will be zeroed before > *any* of the init hooks is called. The first call to oprofile_arch_init() might leave a dirty struct, but as Paul just pointed out the function is aware of it and cleans up the struct during init where necessary. The backtrace callback setup will be reused if arch init fails (seems to be an ugly hack). So it is better not to zero the struct then and keep everything unchanged. -Robert --=20 Advanced Micro Devices, Inc. Operating System Research Center