Re: [PATCH][mmtom] clean up once printk routine

Re: [PATCH][mmtom] clean up once printk routine

[KOSAKI Motohiro]

> == CUT HERE ==
> 
> There are some places to be able to use printk_once instead of hard coding.
> 
> It will help code readability and maintenance.
> This patch doesn't change function's behavior.
> 
> Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
> CC: Dominik Brodowski <linux@dominikbrodowski.net>
> CC: David S. Miller <davem@davemloft.net>
> CC: Ingo Molnar <mingo@elte.hu>
> ---
>  arch/x86/kernel/cpu/common.c  |    8 ++------
>  drivers/net/3c515.c           |    7 ++-----
>  drivers/pcmcia/pcmcia_ioctl.c |    9 +++------
>  3 files changed, 7 insertions(+), 17 deletions(-)

Please separete to three patches ;)

Re: [PATCH][mmtom] clean up once printk routine

[Minchan Kim]

On Tue, 26 May 2009 15:52:32 +0900 (JST)
KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:

> > == CUT HERE ==
> > 
> > There are some places to be able to use printk_once instead of hard coding.
> > 
> > It will help code readability and maintenance.
> > This patch doesn't change function's behavior.
> > 
> > Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
> > CC: Dominik Brodowski <linux@dominikbrodowski.net>
> > CC: David S. Miller <davem@davemloft.net>
> > CC: Ingo Molnar <mingo@elte.hu>
> > ---
> >  arch/x86/kernel/cpu/common.c  |    8 ++------
> >  drivers/net/3c515.c           |    7 ++-----
> >  drivers/pcmcia/pcmcia_ioctl.c |    9 +++------
> >  3 files changed, 7 insertions(+), 17 deletions(-)
> 
> Please separete to three patches ;)

After I listen about things I missed, I will repost it at all once with each patch.
Thanks for comment. Kosaki. :)


-- 
Kinds Regards
Minchan Kim

Re: [PATCH][mmtom] clean up once printk routine

[Pavel Machek]

Hi!

> Yes. There are also some places to be able to use printk_once().
> Are there any place I missed ?
> 
> == CUT HERE ==
> 
> There are some places to be able to use printk_once instead of hard coding.
> 
> It will help code readability and maintenance.
> This patch doesn't change function's behavior.
> 
> Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
> CC: Dominik Brodowski <linux@dominikbrodowski.net>
> CC: David S. Miller <davem@davemloft.net>
> CC: Ingo Molnar <mingo@elte.hu>
> ---
>  arch/x86/kernel/cpu/common.c  |    8 ++------
>  drivers/net/3c515.c           |    7 ++-----
>  drivers/pcmcia/pcmcia_ioctl.c |    9 +++------
>  3 files changed, 7 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 82bec86..dc0f694 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -496,13 +496,9 @@ static void __cpuinit get_cpu_vendor(struct cpuinfo_x86 *c)
>  		}
>  	}
>  
> -	if (!printed) {
> -		printed++;
> -		printk(KERN_ERR
> +	printk_once(KERN_ERR
>  		    "CPU: vendor_id '%s' unknown, using generic init.\n", v);
> -
> -		printk(KERN_ERR "CPU: Your system may be unstable.\n");
> -	}
> +	printk_once(KERN_ERR "CPU: Your system may be unstable.\n");
>

You should delete the variable, right?

Plus, the code now uses two variables instead of one.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH][mmtom] clean up once printk routine

[Minchan Kim]

Hi, Pavel.

On Tue, May 26, 2009 at 5:35 PM, Pavel Machek <pavel@ucw.cz> wrote:
> Hi!
>
>> Yes. There are also some places to be able to use printk_once().
>> Are there any place I missed ?
>>
>> == CUT HERE ==
>>
>> There are some places to be able to use printk_once instead of hard coding.
>>
>> It will help code readability and maintenance.
>> This patch doesn't change function's behavior.
>>
>> Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
>> CC: Dominik Brodowski <linux@dominikbrodowski.net>
>> CC: David S. Miller <davem@davemloft.net>
>> CC: Ingo Molnar <mingo@elte.hu>
>> ---
>>  arch/x86/kernel/cpu/common.c  |    8 ++------
>>  drivers/net/3c515.c           |    7 ++-----
>>  drivers/pcmcia/pcmcia_ioctl.c |    9 +++------
>>  3 files changed, 7 insertions(+), 17 deletions(-)
>>
>> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
>> index 82bec86..dc0f694 100644
>> --- a/arch/x86/kernel/cpu/common.c
>> +++ b/arch/x86/kernel/cpu/common.c
>> @@ -496,13 +496,9 @@ static void __cpuinit get_cpu_vendor(struct cpuinfo_x86 *c)
>>               }
>>       }
>>
>> -     if (!printed) {
>> -             printed++;
>> -             printk(KERN_ERR
>> +     printk_once(KERN_ERR
>>                   "CPU: vendor_id '%s' unknown, using generic init.\n", v);
>> -
>> -             printk(KERN_ERR "CPU: Your system may be unstable.\n");
>> -     }
>> +     printk_once(KERN_ERR "CPU: Your system may be unstable.\n");
>>
>
> You should delete the variable, right?

Yes. you're right.

> Plus, the code now uses two variables instead of one.
>
>                                                                        Pavel

Thanks for pointing me out.
I will repost the patch with your advise.


> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
>



-- 
Thanks,
Minchan Kim

Re: [PATCH][mmtom] clean up once printk routine

[Andrew Morton]

On Tue, 26 May 2009 15:59:43 +0900
Minchan Kim <minchan.kim@gmail.com> wrote:

> On Tue, 26 May 2009 15:52:32 +0900 (JST)
> KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:
> 
> > > == CUT HERE ==
> > > 
> > > There are some places to be able to use printk_once instead of hard coding.
> > > 
> > > It will help code readability and maintenance.
> > > This patch doesn't change function's behavior.
> > > 
> > > Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
> > > CC: Dominik Brodowski <linux@dominikbrodowski.net>
> > > CC: David S. Miller <davem@davemloft.net>
> > > CC: Ingo Molnar <mingo@elte.hu>
> > > ---
> > >  arch/x86/kernel/cpu/common.c  |    8 ++------
> > >  drivers/net/3c515.c           |    7 ++-----
> > >  drivers/pcmcia/pcmcia_ioctl.c |    9 +++------
> > >  3 files changed, 7 insertions(+), 17 deletions(-)
> > 
> > Please separete to three patches ;)
> 
> After I listen about things I missed, I will repost it at all once with each patch.

Yes, that would be better.  But for a trivial little patch like this I
expect we can just merge it and move on.  But please do split up these
multi-subsystem patches in future.

[PATCH][mmtom] clean up printk_once of get_cpu_vendor

[Minchan Kim]

On Tue, 26 May 2009 13:41:34 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Tue, 26 May 2009 15:59:43 +0900
> Minchan Kim <minchan.kim@gmail.com> wrote:
> 
> > On Tue, 26 May 2009 15:52:32 +0900 (JST)
> > KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:
> > 
> > > > == CUT HERE ==
> > > > 
> > > > There are some places to be able to use printk_once instead of hard coding.
> > > > 
> > > > It will help code readability and maintenance.
> > > > This patch doesn't change function's behavior.
> > > > 
> > > > Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
> > > > CC: Dominik Brodowski <linux@dominikbrodowski.net>
> > > > CC: David S. Miller <davem@davemloft.net>
> > > > CC: Ingo Molnar <mingo@elte.hu>
> > > > ---
> > > >  arch/x86/kernel/cpu/common.c  |    8 ++------
> > > >  drivers/net/3c515.c           |    7 ++-----
> > > >  drivers/pcmcia/pcmcia_ioctl.c |    9 +++------
> > > >  3 files changed, 7 insertions(+), 17 deletions(-)
> > > 
> > > Please separete to three patches ;)
> > 
> > After I listen about things I missed, I will repost it at all once with each patch.
> 
> Yes, that would be better.  But for a trivial little patch like this I
> expect we can just merge it and move on.  But please do split up these
> multi-subsystem patches in future.

Thanks. Andrew. 
I confiremd what you merged. 

I modifed get_cpu_vendor's printk-once by Pavel Machek's adivse.
Please, merge with this based on my previous version.

== CUT HERE ==

[PATCH] clean up printk_once of get_cpu_vendor

It remove unnecessary variable and change two static variable
with one.

Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
CC: Ingo Molnar <mingo@elte.hu>
CC: Pavel Machek <pavel@ucw.cz>

---
 arch/x86/kernel/cpu/common.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index dc0f694..c6feb68 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -479,7 +479,6 @@ out:
 static void __cpuinit get_cpu_vendor(struct cpuinfo_x86 *c)
 {
 	char *v = c->x86_vendor_id;
-	static int printed;
 	int i;
 
 	for (i = 0; i < X86_VENDOR_NUM; i++) {
@@ -497,8 +496,8 @@ static void __cpuinit get_cpu_vendor(struct cpuinfo_x86 *c)
 	}
 
 	printk_once(KERN_ERR
-		    "CPU: vendor_id '%s' unknown, using generic init.\n", v);
-	printk_once(KERN_ERR "CPU: Your system may be unstable.\n");
+			"CPU: vendor_id '%s' unknown, using generic init.\n" \
+			"CPU: Your system may be unstable.\n", v);
 
 	c->x86_vendor = X86_VENDOR_UNKNOWN;
 	this_cpu = &default_cpu;
-- 
1.5.4.3








-- 
Kinds Regards
Minchan Kim