All of lore.kernel.org
 help / color / mirror / Atom feed
From: Subrata Modak <subrata@linux.vnet.ibm.com>
To: <x86@kernel.org>
Cc: Sachin P Sant <sachinp@linux.vnet.ibm.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Andi Kleen <andi@firstfloor.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Linux Kernel <linux-kernel@vger.kernel.org>,
	Ingo Molnar <mingo@redhat.com>,
	Subrata Modak <subrata@linux.vnet.ibm.com>,
	Balbir Singh <balbir@linux.vnet.ibm.com>
Subject: [PATCH] Remove indirect variable usage at arch/x86/kernel/tsc.c
Date: Tue, 19 May 2009 23:33:07 +0530	[thread overview]
Message-ID: <20090519180302.603.3124.sendpatchset@subratamodak.linux.ibm.com> (raw)

Hi,

The following warning is generated on compilation:

CC      arch/x86/kernel/tsc.o
arch/x86/kernel/tsc.c: In function 'time_cpufreq_notifier':
arch/x86/kernel/tsc.c:634: warning: 'dummy' may be used uninitialized in this function

However, there seems to be no practical usage of variable 'dummy'
in the following piece of code:

630 static int time_cpufreq_notifier(struct notifier_block *nb, unsigned long val,
631                                 void *data)
632 {
633         struct cpufreq_freqs *freq = data;
634         unsigned long *lpj, dummy;
635 
636         if (cpu_has(&cpu_data(freq->cpu), X86_FEATURE_CONSTANT_TSC))
637                 return 0;
638 
639         lpj = &dummy;
640         if (!(freq->flags & CPUFREQ_CONST_LOOPS))
641 #ifdef CONFIG_SMP
642                 lpj = &cpu_data(freq->cpu).loops_per_jiffy;
643 #else
644         lpj = &boot_cpu_data.loops_per_jiffy;
645 #endif
646 

'lpj' probably will get to point to some address after this if() statement.

647         if (!ref_freq) {
648                 ref_freq = freq->old;
649                 loops_per_jiffy_ref = *lpj;

And, if it does, then "loops_per_jiffy_ref" will have a proper value,
else, even with "lpj = &dummy" will not gurantee "loops_per_jiffy_ref = *lpj"
to have the expected value.

650                 tsc_khz_ref = tsc_khz;
651         }
652         if ((val == CPUFREQ_PRECHANGE  && freq->old < freq->new) ||
653                         (val == CPUFREQ_POSTCHANGE && freq->old > freq->new) ||
654                         (val == CPUFREQ_RESUMECHANGE)) {
655                 *lpj =  cpufreq_scale(loops_per_jiffy_ref, ref_freq, freq->new);
656 
657                 tsc_khz = cpufreq_scale(tsc_khz_ref, ref_freq, freq->new);
658                 if (!(freq->flags & CPUFREQ_CONST_LOOPS))
659                         mark_tsc_unstable("cpufreq changes");
660         }
661 
662         set_cyc2ns_scale(tsc_khz, freq->cpu);
663 
664         return 0;
665 }

Is there any specific reason for 'dummy' to exist in this function ?
If not, i would like to propose the following fix.

Signed-off-by: Subrata Modak <subrata@linux.vnet.ibm.com>
To: <x86@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Balbir Singh <balbir@linux.vnet.ibm.com>
Cc: Sachin P Sant <sachinp@linux.vnet.ibm.com>
Cc: Linux Kernel <linux-kernel@vger.kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Subject: [PATCH] Remove indirect variable usage at arch/x86/kernel/tsc.c
---

--- a/arch/x86/kernel/tsc.c	2009-05-19 00:57:26.000000000 +0530
+++ b/arch/x86/kernel/tsc.c	2009-05-19 21:45:46.000000000 +0530
@@ -631,12 +631,11 @@ static int time_cpufreq_notifier(struct 
 				void *data)
 {
 	struct cpufreq_freqs *freq = data;
-	unsigned long *lpj, dummy;
+	unsigned long *lpj = NULL;
 
 	if (cpu_has(&cpu_data(freq->cpu), X86_FEATURE_CONSTANT_TSC))
 		return 0;
 
-	lpj = &dummy;
 	if (!(freq->flags & CPUFREQ_CONST_LOOPS))
 #ifdef CONFIG_SMP
 		lpj = &cpu_data(freq->cpu).loops_per_jiffy;

---
Regards--
Subrata


             reply	other threads:[~2009-05-19 18:03 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-19 18:03 Subrata Modak [this message]
2009-05-19 18:53 ` [PATCH] Remove indirect variable usage at arch/x86/kernel/tsc.c Frans Pop
2009-05-20  6:10   ` Subrata Modak
2009-05-24 19:42 ` Pavel Machek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090519180302.603.3124.sendpatchset@subratamodak.linux.ibm.com \
    --to=subrata@linux.vnet.ibm.com \
    --cc=andi@firstfloor.org \
    --cc=balbir@linux.vnet.ibm.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=sachinp@linux.vnet.ibm.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.