2009-06-17 13:57:54

by Figo.zhang

[permalink] [raw]
Subject: [PATCH]x86-tsc.c : fix compile warning

fix compile warning:
arch/x86/kernel/tsc.c: In function ‘time_cpufreq_notifier’:
arch/x86/kernel/tsc.c:635: warning: ‘dummy’ may be used uninitialized in
this function

Signed-off-by: Figo.zhang <[email protected]>
---
arch/x86/kernel/tsc.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index ae3180c..e65492a 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -632,7 +632,7 @@ static int time_cpufreq_notifier(struct notifier_block *nb, unsigned long val,
void *data)
{
struct cpufreq_freqs *freq = data;
- unsigned long *lpj, dummy;
+ unsigned long *lpj, dummy = 0;

if (cpu_has(&cpu_data(freq->cpu), X86_FEATURE_CONSTANT_TSC))
return 0;


2009-06-17 14:17:38

by Subrata Modak

[permalink] [raw]
Subject: Re: [PATCH]x86-tsc.c : fix compile warning

> fix compile warning:
> arch/x86/kernel/tsc.c: In function ‘time_cpufreq_notifier’:
> arch/x86/kernel/tsc.c:635: warning: ‘dummy’ may be used uninitialized in
> this function
>
> Signed-off-by: Figo.zhang <[email protected]>

Hello Figo,

Please refer to the following links on discussion on this issue earlier:
http://lkml.org/lkml/2009/5/19/312
http://lkml.org/lkml/2009/5/19/301
http://lkml.org/lkml/2009/5/20/23

I tried to fix it in some other way.

Regards--
Subrata

> ---
> arch/x86/kernel/tsc.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index ae3180c..e65492a 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -632,7 +632,7 @@ static int time_cpufreq_notifier(struct
> notifier_block *nb, unsigned long val,
> void *data)
> {
> struct cpufreq_freqs *freq = data;
> - unsigned long *lpj, dummy;
> + unsigned long *lpj, dummy = 0;
>
> if (cpu_has(&cpu_data(freq->cpu), X86_FEATURE_CONSTANT_TSC))
> return 0;
>

2009-06-17 14:21:10

by Figo.zhang

[permalink] [raw]
Subject: Re: [PATCH]x86-tsc.c : fix compile warning

On Wed, 2009-06-17 at 19:47 +0530, Subrata Modak wrote:
> > fix compile warning:
> > arch/x86/kernel/tsc.c: In function ‘time_cpufreq_notifier’:
> > arch/x86/kernel/tsc.c:635: warning: ‘dummy’ may be used uninitialized in
> > this function
> >
> > Signed-off-by: Figo.zhang <[email protected]>
>
> Hello Figo,
>
> Please refer to the following links on discussion on this issue earlier:
> http://lkml.org/lkml/2009/5/19/312
> http://lkml.org/lkml/2009/5/19/301
> http://lkml.org/lkml/2009/5/20/23
>
> I tried to fix it in some other way.

hi Subrata,

That is ok.

Best Regards,
Figo.zhang
>
> Regards--
> Subrata
>
> > ---
> > arch/x86/kernel/tsc.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> > index ae3180c..e65492a 100644
> > --- a/arch/x86/kernel/tsc.c
> > +++ b/arch/x86/kernel/tsc.c
> > @@ -632,7 +632,7 @@ static int time_cpufreq_notifier(struct
> > notifier_block *nb, unsigned long val,
> > void *data)
> > {
> > struct cpufreq_freqs *freq = data;
> > - unsigned long *lpj, dummy;
> > + unsigned long *lpj, dummy = 0;
> >
> > if (cpu_has(&cpu_data(freq->cpu), X86_FEATURE_CONSTANT_TSC))
> > return 0;
> >
>
>

2009-06-17 15:28:26

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH]x86-tsc.c : fix compile warning


* Subrata Modak <[email protected]> wrote:

> > fix compile warning:
> > arch/x86/kernel/tsc.c: In function ‘time_cpufreq_notifier’:
> > arch/x86/kernel/tsc.c:635: warning: ‘dummy’ may be used uninitialized in
> > this function
> >
> > Signed-off-by: Figo.zhang <[email protected]>
>
> Hello Figo,
>
> Please refer to the following links on discussion on this issue earlier:
> http://lkml.org/lkml/2009/5/19/312
> http://lkml.org/lkml/2009/5/19/301
> http://lkml.org/lkml/2009/5/20/23
>
> I tried to fix it in some other way.

Ah, and you fixed in what a superior way: you improved the code in
the process :-) This is how warnings should be fixed really.

Applied.

We apparently missed it when you sent it in May - generally feel
free to re-send patches if you dont see a commit notification within
a few days.

Thanks,

Ingo

2009-06-17 16:11:42

by Subrata Modak

[permalink] [raw]
Subject: Re: [PATCH]x86-tsc.c : fix compile warning

On Wed, 2009-06-17 at 17:28 +0200, Ingo Molnar wrote:
> * Subrata Modak <[email protected]> wrote:
>
> > > fix compile warning:
> > > arch/x86/kernel/tsc.c: In function ‘time_cpufreq_notifier’:
> > > arch/x86/kernel/tsc.c:635: warning: ‘dummy’ may be used uninitialized in
> > > this function
> > >
> > > Signed-off-by: Figo.zhang <[email protected]>
> >
> > Hello Figo,
> >
> > Please refer to the following links on discussion on this issue earlier:
> > http://lkml.org/lkml/2009/5/19/312
> > http://lkml.org/lkml/2009/5/19/301
> > http://lkml.org/lkml/2009/5/20/23
> >
> > I tried to fix it in some other way.
>
> Ah, and you fixed in what a superior way: you improved the code in
> the process :-) This is how warnings should be fixed really.

Thanks Ingo. I would do whenever i find again one.

>
> Applied.
>
> We apparently missed it when you sent it in May - generally feel
> free to re-send patches if you dont see a commit notification within
> a few days.

My mistake. I did not keep track.

Regards--
Subrata

>
> Thanks,
>
> Ingo

2009-06-17 16:12:58

by Frans Pop

[permalink] [raw]
Subject: Re: [PATCH]x86-tsc.c : fix compile warning

On Wednesday 17 June 2009, Ingo Molnar wrote:
> Ah, and you fixed in what a superior way: you improved the code in
> the process :-) This is how warnings should be fixed really.

Hmm. Did you also see Pavel's reply to that patch [1]:
! But that's a bug to be fixed, I'd say? ... actually I believe you are
! introducing a bug here. Yes, old code would put random numbers in
! loops_per_jiffy_ref for !CPUFREQ_CONST_LOOPS, but you are introducing
! oops there.

Was his comment incorrect?

Cheers,
FJP

[1] http://lkml.org/lkml/2009/5/24/159

2009-06-17 16:23:44

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH]x86-tsc.c : fix compile warning


* Frans Pop <[email protected]> wrote:

> On Wednesday 17 June 2009, Ingo Molnar wrote:
> > Ah, and you fixed in what a superior way: you improved the code in
> > the process :-) This is how warnings should be fixed really.
>
> Hmm. Did you also see Pavel's reply to that patch [1]:
> ! But that's a bug to be fixed, I'd say? ... actually I believe you are
> ! introducing a bug here. Yes, old code would put random numbers in
> ! loops_per_jiffy_ref for !CPUFREQ_CONST_LOOPS, but you are introducing
> ! oops there.
>
> Was his comment incorrect?
>
> Cheers,
> FJP
>
> [1] http://lkml.org/lkml/2009/5/24/159

hm, Pavel seems right - i missed that. Subrata, mind sending an
updated patch?

Thanks,

Ingo

2009-06-17 16:55:29

by Subrata Modak

[permalink] [raw]
Subject: Re: [PATCH]x86-tsc.c : fix compile warning

On Wed, 2009-06-17 at 18:23 +0200, Ingo Molnar wrote:
> * Frans Pop <[email protected]> wrote:
>
> > On Wednesday 17 June 2009, Ingo Molnar wrote:
> > > Ah, and you fixed in what a superior way: you improved the code in
> > > the process :-) This is how warnings should be fixed really.
> >
> > Hmm. Did you also see Pavel's reply to that patch [1]:
> > ! But that's a bug to be fixed, I'd say? ... actually I believe you are
> > ! introducing a bug here. Yes, old code would put random numbers in
> > ! loops_per_jiffy_ref for !CPUFREQ_CONST_LOOPS, but you are introducing
> > ! oops there.
> >
> > Was his comment incorrect?
> >
> > Cheers,
> > FJP
> >
> > [1] http://lkml.org/lkml/2009/5/24/159
>
> hm, Pavel seems right - i missed that. Subrata, mind sending an
> updated patch?

Hmmm. I would try doing that soon.

Regards--
Subrata

>
> Thanks,
>
> Ingo

2009-06-18 07:28:14

by Balbir Singh

[permalink] [raw]
Subject: Re: [PATCH]x86-tsc.c : fix compile warning

* Ingo Molnar <[email protected]> [2009-06-17 18:23:03]:

>
> * Frans Pop <[email protected]> wrote:
>
> > On Wednesday 17 June 2009, Ingo Molnar wrote:
> > > Ah, and you fixed in what a superior way: you improved the code in
> > > the process :-) This is how warnings should be fixed really.
> >
> > Hmm. Did you also see Pavel's reply to that patch [1]:
> > ! But that's a bug to be fixed, I'd say? ... actually I believe you are
> > ! introducing a bug here. Yes, old code would put random numbers in
> > ! loops_per_jiffy_ref for !CPUFREQ_CONST_LOOPS, but you are introducing
> > ! oops there.
> >
> > Was his comment incorrect?
> >
> > Cheers,
> > FJP
> >
> > [1] http://lkml.org/lkml/2009/5/24/159
>
> hm, Pavel seems right - i missed that. Subrata, mind sending an
> updated patch?
>

Hi, Ingo,

Yes, it does seem that we'll oops at *lpj, but to be honest the code
is badly written, ideally the CONFIG_SMP part should be abstracted
out, having that in a if loop makes reading it time consuming and
kills a few neuro cells each time.


--
Balbir

2009-06-18 08:48:20

by Subrata Modak

[permalink] [raw]
Subject: Re: [PATCH]x86-tsc.c : fix compile warning

On Thu, 2009-06-18 at 12:57 +0530, Balbir Singh wrote:
> * Ingo Molnar <[email protected]> [2009-06-17 18:23:03]:
>
> >
> > * Frans Pop <[email protected]> wrote:
> >
> > > On Wednesday 17 June 2009, Ingo Molnar wrote:
> > > > Ah, and you fixed in what a superior way: you improved the code in
> > > > the process :-) This is how warnings should be fixed really.
> > >
> > > Hmm. Did you also see Pavel's reply to that patch [1]:
> > > ! But that's a bug to be fixed, I'd say? ... actually I believe you are
> > > ! introducing a bug here. Yes, old code would put random numbers in
> > > ! loops_per_jiffy_ref for !CPUFREQ_CONST_LOOPS, but you are introducing
> > > ! oops there.
> > >
> > > Was his comment incorrect?
> > >
> > > Cheers,
> > > FJP
> > >
> > > [1] http://lkml.org/lkml/2009/5/24/159
> >
> > hm, Pavel seems right - i missed that. Subrata, mind sending an
> > updated patch?
> >
>
> Hi, Ingo,
>
> Yes, it does seem that we'll oops at *lpj, but to be honest the code
> is badly written, ideally the CONFIG_SMP part should be abstracted
> out, having that in a if loop makes reading it time consuming and
> kills a few neuro cells each time.

Balbir/Ingo,

I find this fixed in todayś Linus´s git tree. dummy has been removed,
and, so does lpj initialization taking place properly.

631 static int time_cpufreq_notifier(struct notifier_block *nb, unsigned
long val,
632 void *data)
633 {
634 struct cpufreq_freqs *freq = data;
635 unsigned long *lpj;
636
637 if (cpu_has(&cpu_data(freq->cpu), X86_FEATURE_CONSTANT_TSC))
638 return 0;
639
640 lpj = &boot_cpu_data.loops_per_jiffy;

this gets initialized here for the first time on any system.

641 #ifdef CONFIG_SMP
642 if (!(freq->flags & CPUFREQ_CONST_LOOPS))
643 lpj = &cpu_data(freq->cpu).loops_per_jiffy;
644 #endif

And gets re-assigned if CONFIG_SMP.

Regards--
Subrata

>
>