Re: [PATCH] powerpc: Silence timebase sync code

2008-11-18 Thread Trent Piepho 
On Tue, 18 Nov 2008, Michael Ellerman wrote:
 On Mon, 2008-11-17 at 14:22 -0800, Trent Piepho wrote:
 On Mon, 17 Nov 2008, Kumar Gala wrote:
 On Nov 17, 2008, at 3:58 PM, Trent Piepho wrote:

 It's over a dozen lines of output and doesn't appear to provide any useful
 information.  Even after looking at the code, I'm in the dark about what
 score 299, offset 250 means.

 Signed-off-by: Trent Piepho [EMAIL PROTECTED]
 ---
 +++ b/arch/powerpc/kernel/smp-tbsync.c
 @@ -113,7 +113,7 @@ void __devinit smp_generic_give_timebase(void)
 {
  int i, score, score2, old, min=0, max=5000, offset=1000;

 -  printk(Synchronizing timebase\n);
 +  pr_info(Synchronizing timebase\n);

 I think its useful to leave this as a printk.

 #define pr_info(fmt, arg...) \
  printk(KERN_INFO fmt, ##arg)

 Isn't printk with no level tag the same as KERN_INFO?

 Stuff like this should IMHO be printk(KERN_DEBUG ..)

 That way it will show up in the log as long as you boot with 'debug' on
 your command line, it doesn't require a kernel recompile to turn on. And
 at the same time it doesn't spam the boot log for a normal boot.

Originally my patched changed it to debug, but Kumar requested I keep it at
info level.  The timebase sync code might hang or be slow, so it's nice to
give a status output before doing it.  It seems just as good as most of the
info printks during boot.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] powerpc: Silence timebase sync code

2008-11-18 Thread Benjamin Herrenschmidt 
On Mon, 2008-11-17 at 13:58 -0800, Trent Piepho wrote:
 It's over a dozen lines of output and doesn't appear to provide any useful
 information.  Even after looking at the code, I'm in the dark about what
 score 299, offset 250 means.

Hah ! I had almost the same patch in my pile for some time :-)

 Signed-off-by: Trent Piepho [EMAIL PROTECTED]

Acked-by: Benjamin Herrenschmidt [EMAIL PROTECTED]

 ---
  arch/powerpc/kernel/smp-tbsync.c |   12 
  1 files changed, 4 insertions(+), 8 deletions(-)
 
 diff --git a/arch/powerpc/kernel/smp-tbsync.c 
 b/arch/powerpc/kernel/smp-tbsync.c
 index bc892e6..b590135 100644
 --- a/arch/powerpc/kernel/smp-tbsync.c
 +++ b/arch/powerpc/kernel/smp-tbsync.c
 @@ -113,7 +113,7 @@ void __devinit smp_generic_give_timebase(void)
  {
   int i, score, score2, old, min=0, max=5000, offset=1000;
  
 - printk(Synchronizing timebase\n);
 + pr_info(Synchronizing timebase\n);
  
   /* if this fails then this kernel won't work anyway... */
   tbsync = kzalloc( sizeof(*tbsync), GFP_KERNEL );
 @@ -123,14 +123,10 @@ void __devinit smp_generic_give_timebase(void)
   while (!tbsync-ack)
   barrier();
  
 - printk(Got ack\n);
 -
   /* binary search */
   for (old = -1; old != offset ; offset = (min+max) / 2) {
   score = start_contest(kSetAndTest, offset, NUM_ITER);
  
 - printk(score %d, offset %d\n, score, offset );
 -
   if( score  0 )
   max = offset;
   else
 @@ -140,8 +136,8 @@ void __devinit smp_generic_give_timebase(void)
   score = start_contest(kSetAndTest, min, NUM_ITER);
   score2 = start_contest(kSetAndTest, max, NUM_ITER);
  
 - printk(Min %d (score %d), Max %d (score %d)\n,
 -min, score, max, score2);
 + pr_debug(Min %d (score %d), Max %d (score %d)\n,
 +  min, score, max, score2);
   score = abs(score);
   score2 = abs(score2);
   offset = (score  score2) ? min : max;
 @@ -155,7 +151,7 @@ void __devinit smp_generic_give_timebase(void)
   if (score2 = score || score2  20)
   break;
   }
 - printk(Final offset: %d (%d/%d)\n, offset, score2, NUM_ITER );
 + pr_debug(Final offset: %d (%d/%d)\n, offset, score2, NUM_ITER);
  
   /* exiting */
   tbsync-cmd = kExit;

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] powerpc: Silence timebase sync code

2008-11-18 Thread Kumar Gala 


On Nov 17, 2008, at 4:22 PM, Trent Piepho wrote:


On Mon, 17 Nov 2008, Kumar Gala wrote:

On Nov 17, 2008, at 3:58 PM, Trent Piepho wrote:

It's over a dozen lines of output and doesn't appear to provide  
any useful
information.  Even after looking at the code, I'm in the dark  
about what

score 299, offset 250 means.

Signed-off-by: Trent Piepho [EMAIL PROTECTED]
---
arch/powerpc/kernel/smp-tbsync.c |   12 
1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/kernel/smp-tbsync.c
b/arch/powerpc/kernel/smp-tbsync.c
index bc892e6..b590135 100644
--- a/arch/powerpc/kernel/smp-tbsync.c
+++ b/arch/powerpc/kernel/smp-tbsync.c
@@ -113,7 +113,7 @@ void __devinit smp_generic_give_timebase(void)
{
int i, score, score2, old, min=0, max=5000, offset=1000;

-   printk(Synchronizing timebase\n);
+   pr_info(Synchronizing timebase\n);


I think its useful to leave this as a printk.


#define pr_info(fmt, arg...) \
printk(KERN_INFO fmt, ##arg)

Isn't printk with no level tag the same as KERN_INFO?


oops, sorry.. read the pr_info() as pr_debug()

- k
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] powerpc: Silence timebase sync code

2008-11-18 Thread Paul Mackerras 
Trent Piepho writes:

 It's over a dozen lines of output and doesn't appear to provide any useful
 information.  Even after looking at the code, I'm in the dark about what
 score 299, offset 250 means.

I already put a very similar patch from Ben Herrenschmidt into my
powerpc.git tree next branch, back on 5 November.

Paul.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] powerpc: Silence timebase sync code

2008-11-17 Thread Kumar Gala 


On Nov 17, 2008, at 3:58 PM, Trent Piepho wrote:

It's over a dozen lines of output and doesn't appear to provide any  
useful
information.  Even after looking at the code, I'm in the dark about  
what

score 299, offset 250 means.

Signed-off-by: Trent Piepho [EMAIL PROTECTED]
---
arch/powerpc/kernel/smp-tbsync.c |   12 
1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/kernel/smp-tbsync.c b/arch/powerpc/kernel/ 
smp-tbsync.c

index bc892e6..b590135 100644
--- a/arch/powerpc/kernel/smp-tbsync.c
+++ b/arch/powerpc/kernel/smp-tbsync.c
@@ -113,7 +113,7 @@ void __devinit smp_generic_give_timebase(void)
{
int i, score, score2, old, min=0, max=5000, offset=1000;

-   printk(Synchronizing timebase\n);
+   pr_info(Synchronizing timebase\n);


I think its useful to leave this as a printk.

- k
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] powerpc: Silence timebase sync code

2008-11-17 Thread Trent Piepho 
On Mon, 17 Nov 2008, Kumar Gala wrote:
 On Nov 17, 2008, at 3:58 PM, Trent Piepho wrote:

 It's over a dozen lines of output and doesn't appear to provide any useful
 information.  Even after looking at the code, I'm in the dark about what
 score 299, offset 250 means.
 
 Signed-off-by: Trent Piepho [EMAIL PROTECTED]
 ---
 arch/powerpc/kernel/smp-tbsync.c |   12 
 1 files changed, 4 insertions(+), 8 deletions(-)
 
 diff --git a/arch/powerpc/kernel/smp-tbsync.c 
 b/arch/powerpc/kernel/smp-tbsync.c
 index bc892e6..b590135 100644
 --- a/arch/powerpc/kernel/smp-tbsync.c
 +++ b/arch/powerpc/kernel/smp-tbsync.c
 @@ -113,7 +113,7 @@ void __devinit smp_generic_give_timebase(void)
 {
  int i, score, score2, old, min=0, max=5000, offset=1000;
 
 -printk(Synchronizing timebase\n);
 +pr_info(Synchronizing timebase\n);

 I think its useful to leave this as a printk.

#define pr_info(fmt, arg...) \
 printk(KERN_INFO fmt, ##arg)

Isn't printk with no level tag the same as KERN_INFO?
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] powerpc: Silence timebase sync code

2008-11-17 Thread Michael Ellerman 
On Mon, 2008-11-17 at 14:22 -0800, Trent Piepho wrote:
 On Mon, 17 Nov 2008, Kumar Gala wrote:
  On Nov 17, 2008, at 3:58 PM, Trent Piepho wrote:
 
  It's over a dozen lines of output and doesn't appear to provide any useful
  information.  Even after looking at the code, I'm in the dark about what
  score 299, offset 250 means.
  
  Signed-off-by: Trent Piepho [EMAIL PROTECTED]
  ---
  arch/powerpc/kernel/smp-tbsync.c |   12 
  1 files changed, 4 insertions(+), 8 deletions(-)
  
  diff --git a/arch/powerpc/kernel/smp-tbsync.c 
  b/arch/powerpc/kernel/smp-tbsync.c
  index bc892e6..b590135 100644
  --- a/arch/powerpc/kernel/smp-tbsync.c
  +++ b/arch/powerpc/kernel/smp-tbsync.c
  @@ -113,7 +113,7 @@ void __devinit smp_generic_give_timebase(void)
  {
   int i, score, score2, old, min=0, max=5000, offset=1000;
  
  -  printk(Synchronizing timebase\n);
  +  pr_info(Synchronizing timebase\n);
 
  I think its useful to leave this as a printk.
 
 #define pr_info(fmt, arg...) \
  printk(KERN_INFO fmt, ##arg)
 
 Isn't printk with no level tag the same as KERN_INFO?

Stuff like this should IMHO be printk(KERN_DEBUG ..)

That way it will show up in the log as long as you boot with 'debug' on
your command line, it doesn't require a kernel recompile to turn on. And
at the same time it doesn't spam the boot log for a normal boot.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person


signature.asc
Description: This is a digitally signed message part
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev