linux-kernel - Re: [PATCH] mcheck/therm_throt: Replace printk(KERN_XX...) with pr

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-next>] [day] [month] [year] [list]

Message-ID: <20160201082412.GA15735@gmail.com>
Date:	Mon, 1 Feb 2016 09:24:12 +0100
From:	Ingo Molnar <mingo@...nel.org>
To:	Chen Yucong <slaoub@...il.com>
Cc:	hpa@...or.com, tony.luck@...el.com, bp@...en8.de, x86@...nel.org,
	linux-edac@...r.kernel.org, linux-kernel@...r.kernel.org,
	Thomas Gleixner <tglx@...utronix.de>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>
Subject: Re: [PATCH] mcheck/therm_throt: Replace printk(KERN_XX...) with
 pr_xx(...)


(changed the subject to be lkml compatible. Mail quoted below.)

* Ingo Molnar <mingo@...nel.org> wrote:

> 
> * Chen Yucong <slaoub@...il.com> wrote:
> 
> > This patch also has replaced one more printk_once() with pr_info_once().
> > 
> > Signed-off-by: Chen Yucong <slaoub@...il.com>
> > ---
> >  arch/x86/kernel/cpu/mcheck/therm_throt.c | 13 ++++++-------
> >  1 file changed, 6 insertions(+), 7 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/cpu/mcheck/therm_throt.c b/arch/x86/kernel/cpu/mcheck/therm_throt.c
> > index 2c5aaf8..6cc5eed 100644
> > --- a/arch/x86/kernel/cpu/mcheck/therm_throt.c
> > +++ b/arch/x86/kernel/cpu/mcheck/therm_throt.c
> > @@ -190,7 +190,7 @@ static int therm_throt_process(bool new_event, int event, int level)
> >  	/* if we just entered the thermal event */
> >  	if (new_event) {
> >  		if (event == THERMAL_THROTTLING_EVENT)
> > -			printk(KERN_CRIT "CPU%d: %s temperature above threshold, cpu clock throttled (total events = %lu)\n",
> > +			pr_crit("CPU%d: %s temperature above threshold, cpu clock throttled (total events = %lu)\n",
> >  				this_cpu,
> >  				level == CORE_LEVEL ? "Core" : "Package",
> >  				state->count);
> > @@ -198,7 +198,7 @@ static int therm_throt_process(bool new_event, int event, int level)
> >  	}
> >  	if (old_event) {
> >  		if (event == THERMAL_THROTTLING_EVENT)
> > -			printk(KERN_INFO "CPU%d: %s temperature/speed normal\n",
> > +			pr_info("CPU%d: %s temperature/speed normal\n",
> >  				this_cpu,
> >  				level == CORE_LEVEL ? "Core" : "Package");
> >  		return 1;
> > @@ -417,8 +417,7 @@ static void intel_thermal_interrupt(void)
> >  
> >  static void unexpected_thermal_interrupt(void)
> >  {
> > -	printk(KERN_ERR "CPU%d: Unexpected LVT thermal interrupt!\n",
> > -			smp_processor_id());
> > +	pr_err("CPU%d: Unexpected LVT thermal interrupt!\n", smp_processor_id());
> >  }
> >  
> >  static void (*smp_thermal_vector)(void) = unexpected_thermal_interrupt;
> > @@ -499,7 +498,7 @@ void intel_init_thermal(struct cpuinfo_x86 *c)
> >  
> >  	if ((l & MSR_IA32_MISC_ENABLE_TM1) && (h & APIC_DM_SMI)) {
> >  		if (system_state == SYSTEM_BOOTING)
> > -			printk(KERN_DEBUG "CPU%d: Thermal monitoring handled by SMI\n", cpu);
> > +			pr_debug("CPU%d: Thermal monitoring handled by SMI\n", cpu);
> >  		return;
> >  	}
> >  
> > @@ -557,8 +556,8 @@ void intel_init_thermal(struct cpuinfo_x86 *c)
> >  	l = apic_read(APIC_LVTTHMR);
> >  	apic_write(APIC_LVTTHMR, l & ~APIC_LVT_MASKED);
> >  
> > -	printk_once(KERN_INFO "CPU0: Thermal monitoring enabled (%s)\n",
> > -		       tm2 ? "TM2" : "TM1");
> > +	pr_info_once("CPU0: Thermal monitoring enabled (%s)\n",
> > +		      tm2 ? "TM2" : "TM1");
> >  
> >  	/* enable thermal throttle processing */
> >  	atomic_set(&therm_throt_en, 1);
> 
> So there are 37,000+ printk() calls in the kernel and 900+ ones in arch/x86/ 
> alone! Plus even after this patch there's 20 more printk()s left in 
> arch/x86/kernel/cpu/mcheck/...
> 
> We don't want to create a churn of 10,000+ commits to convert them to pr_*() 
> facilities...
> 
> So we don't apply such 'conversion' patches unless it's done for a whole subsystem 
> and done as part of a larger work with good reason, or if it's done as part of 
> completely new facilities/drivers - and is done consistently.
> 
> Even then it's dubious to convert: people keep re-adding printk()s (which is a 
> perfectly fine facility), which generates never ending churn ...
> 
> Thanks,
> 
> 	Ingo