Messages in this thread |  | | Subject | Re: [PATCH] printk: Add best-effort printk() buffering. | From | Joe Perches <> | Date | Wed, 03 May 2017 02:30:11 -0700 |
| |
On Wed, 2017-05-03 at 15:21 +0900, Tetsuo Handa wrote: > Joe Perches wrote: > > On Sun, 2017-04-30 at 22:54 +0900, Tetsuo Handa wrote: > > > Sometimes we want to printk() multiple lines in a group without being > > > disturbed by concurrent printk() from interrupts and/or other threads. > > > For example, mixed printk() output of multiple thread's dump makes it > > > hard to interpret. > > > > > > This patch introduces fixed-sized statically allocated buffers for > > > buffering printk() output for each thread/context in best effort > > > (i.e. up to PAGE_SIZE bytes, up to 16 concurrent printk() users). > > > > [] > > > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > > > > [] > > > +#define MAX_PRINTK_BUFFERS 16 > > > +static struct printk_buffer { > > > + unsigned long context; /* printk_context() */ > > > + unsigned int nested; > > > + unsigned int used; /* Valid bytes in buf[]. */ > > > + char buf[PAGE_SIZE]; > > > +} printk_buffers[MAX_PRINTK_BUFFERS]; > > > > Perhaps these buffers could be acquired by > > alloc_page rather than be static structures and > > the sizeof buf[PAGE_SIZE] should be reduced by > > sizeof(unsigned long) + > > sizeof(unsigned int) + > > sizeof(unsigned int) > > so that struct printk_buffers is exactly > > PAGE_SIZE. > > When should the buffers be allocated? If upon boot, there will be little > difference. If the first time each buffer is needed, we introduce a risk > of failing to allocate memory using alloc_page(GFP_ATOMIC | __GFP_NOWARN) > and a risk of stack overflow during alloc_page() because printk() has to be > prepared for being called from stack-tight situation.
This is supposed to be best effort anyway.
> Also, while dynamic allocation can allow linked list of the buffer, we > will need to introduce a lock for traversing the list, which might become > more expensive than walking fixed-sized array of the buffer.
Shouldn't matter as this is supposed to be best effort and any printk is already quite expensive and not fast-path.
> We could avoid list traversal by passing "struct printk_buffer" argument, > but since there are so many functions which expect pr_cont() behavior, > scattering "struct printk_buffer" argument is a big patch.
There would be possible to add pid instead.
> Thus, I think fixed-sized statically allocated buffers is the most > reasonable choice. Using a CONFIG_ option for controlling how many pages > should be allocated for "struct printk_buffer" might make sense for systems > with little RAM.
Simpler, but not better.
|  |