[PATCH] percpu: use KERN_CONT in pcpu_dump_alloc_info()

[PATCH] percpu: use KERN_CONT in pcpu_dump_alloc_info()

[Tejun Heo]

>From cb129820f1e6ccf309510f4eb28df45cb0742005 Mon Sep 17 00:00:00 2001
From: Tejun Heo <tj@kernel.org>
Date: Thu, 29 Mar 2012 09:45:58 -0700

pcpu_dump_alloc_info() was printing continued lines without KERN_CONT.
Use it.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Kay Sievers <kay.sievers@vrfy.org>
---
Committed to percpu/for-3.4-fixes.

Thanks.

 mm/percpu.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/mm/percpu.c b/mm/percpu.c
index f47af91..f921fdf 100644
--- a/mm/percpu.c
+++ b/mm/percpu.c
@@ -1132,20 +1132,20 @@ static void pcpu_dump_alloc_info(const char *lvl,
 		for (alloc_end += gi->nr_units / upa;
 		     alloc < alloc_end; alloc++) {
 			if (!(alloc % apl)) {
-				printk("\n");
+				printk(KERN_CONT "\n");
 				printk("%spcpu-alloc: ", lvl);
 			}
-			printk("[%0*d] ", group_width, group);
+			printk(KERN_CONT "[%0*d] ", group_width, group);
 
 			for (unit_end += upa; unit < unit_end; unit++)
 				if (gi->cpu_map[unit] != NR_CPUS)
-					printk("%0*d ", cpu_width,
+					printk(KERN_CONT "%0*d ", cpu_width,
 					       gi->cpu_map[unit]);
 				else
-					printk("%s ", empty_str);
+					printk(KERN_CONT "%s ", empty_str);
 		}
 	}
-	printk("\n");
+	printk(KERN_CONT "\n");
 }
 
 /**
-- 
1.7.7.3

Re: [PATCH] percpu: use KERN_CONT in pcpu_dump_alloc_info()

[Joe Perches]

On Thu, 2012-03-29 at 09:48 -0700, Tejun Heo wrote:
> >From cb129820f1e6ccf309510f4eb28df45cb0742005 Mon Sep 17 00:00:00 2001
> From: Tejun Heo <tj@kernel.org>
> Date: Thu, 29 Mar 2012 09:45:58 -0700
> 
> pcpu_dump_alloc_info() was printing continued lines without KERN_CONT.
> Use it.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Reported-by: Kay Sievers <kay.sievers@vrfy.org>
> ---
> Committed to percpu/for-3.4-fixes.
> 
> Thanks.
> 
>  mm/percpu.c |   10 +++++-----
>  1 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/percpu.c b/mm/percpu.c
> index f47af91..f921fdf 100644
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -1132,20 +1132,20 @@ static void pcpu_dump_alloc_info(const char *lvl,
>  		for (alloc_end += gi->nr_units / upa;
>  		     alloc < alloc_end; alloc++) {
>  			if (!(alloc % apl)) {
> -				printk("\n");
> +				printk(KERN_CONT "\n");
>  				printk("%spcpu-alloc: ", lvl);
>  			}
> -			printk("[%0*d] ", group_width, group);
> +			printk(KERN_CONT "[%0*d] ", group_width, group);
>  
>  			for (unit_end += upa; unit < unit_end; unit++)
>  				if (gi->cpu_map[unit] != NR_CPUS)
> -					printk("%0*d ", cpu_width,
> +					printk(KERN_CONT "%0*d ", cpu_width,
>  					       gi->cpu_map[unit]);
>  				else
> -					printk("%s ", empty_str);
> +					printk(KERN_CONT "%s ", empty_str);
>  		}
>  	}
> -	printk("\n");
> +	printk(KERN_CONT "\n");
>  }

Perhaps it's better to try to print lines
in fewer printks?  Maybe use a line buffer?

The first line could be newline terminated.
"if (alloc)" tests for the individual newlines could be added.
Why aren't the group widths on separate lines?
All of the cpu_widths are on the same line as the parent group
even if that exceeds apl.  Is that what's desired?

Re: [PATCH] percpu: use KERN_CONT in pcpu_dump_alloc_info()

[Tejun Heo]

Hello, Joe.

On Thu, Mar 29, 2012 at 11:10 AM, Joe Perches <joe@perches.com> wrote:
> Perhaps it's better to try to print lines
> in fewer printks?  Maybe use a line buffer?

What practical difference would that make?

> The first line could be newline terminated.
> "if (alloc)" tests for the individual newlines could be added.
> Why aren't the group widths on separate lines?
> All of the cpu_widths are on the same line as the parent group
> even if that exceeds apl.  Is that what's desired?

I frankly don't remember. I wanted something easy on the eye during
the development to verify the config results and it worked well enough
for me. If you can simplify it, please go ahead and submit a patch.

Thanks.

-- 
tejun

Re: [PATCH] percpu: use KERN_CONT in pcpu_dump_alloc_info()

[Joe Perches]

On Thu, 2012-03-29 at 12:51 -0700, Tejun Heo wrote:
> Hello, Joe.

Hello Tejun.

> On Thu, Mar 29, 2012 at 11:10 AM, Joe Perches <joe@perches.com> wrote:
> > Perhaps it's better to try to print lines
> > in fewer printks?  Maybe use a line buffer?
> What practical difference would that make?

It'd be just for avoidance of any other possible
interleaving from other threads.

It's an __init function so that's not too likely,
but if it every happens...

Re: [PATCH] percpu: use KERN_CONT in pcpu_dump_alloc_info()

[Tejun Heo]

Hello,

On Thu, Mar 29, 2012 at 1:06 PM, Joe Perches <joe@perches.com> wrote:
> It'd be just for avoidance of any other possible
> interleaving from other threads.
>
> It's an __init function so that's not too likely,
> but if it every happens...

Ah, okay.  Forgot about that.  Yeah, the kernel is single threaded at
that point, so it shouldn't matter. The code is rather complex so
something simpler would be nice, I agree.

Another thing is that we seem to have structured logging coming our
way and maybe userspace can deal with racing continuing printks? Not
sure how different streams could be distinguished tho. Maybe the
printing task can be used to group them? Kay, is this something
structured logging would solve?

Thanks.

-- 
tejun

Re: [PATCH] percpu: use KERN_CONT in pcpu_dump_alloc_info()

[Kay Sievers]

On Thu, Mar 29, 2012 at 22:11, Tejun Heo <tj@kernel.org> wrote:
> On Thu, Mar 29, 2012 at 1:06 PM, Joe Perches <joe@perches.com> wrote:
>> It'd be just for avoidance of any other possible
>> interleaving from other threads.
>>
>> It's an __init function so that's not too likely,
>> but if it every happens...
>
> Ah, okay.  Forgot about that.  Yeah, the kernel is single threaded at
> that point, so it shouldn't matter. The code is rather complex so
> something simpler would be nice, I agree.
>
> Another thing is that we seem to have structured logging coming our
> way and maybe userspace can deal with racing continuing printks? Not
> sure how different streams could be distinguished tho. Maybe the
> printing task can be used to group them? Kay, is this something
> structured logging would solve?

The kernel's printk() uses a static scratch buffer of 1024 chars. The
prototype for the printk() with facility and structure support I have,
does a bit better and more reliable 'buffered IO' than the current
one.

If interleaved printk messages are still a problem, we could allocate
a per-cpu scratch buffer and all continuation lines which stay on the
same CPU would share the same buffer before it is committed to the
global log buffer.

Kay

Re: [PATCH] percpu: use KERN_CONT in pcpu_dump_alloc_info()

[Joe Perches]

On Sun, 2012-04-01 at 15:29 +0200, Kay Sievers wrote:
> The kernel's printk() uses a static scratch buffer of 1024 chars. The
> prototype for the printk() with facility and structure support I have,
> does a bit better and more reliable 'buffered IO' than the current
> one.

Can you send me a link to your code please?