lkml.org 
[lkml]   [2018]   [Oct]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] printk: inject caller information into the body of message
    On Tue 2018-10-09 05:48:33, Tetsuo Handa wrote:
    > On 2018/10/09 1:03, Petr Mladek wrote:
    > > On Mon 2018-10-08 19:31:58, Tetsuo Handa wrote:
    > >> A structure named "struct printk_buffer" is introduced for buffering
    > >> up to LOG_LINE_MAX bytes of printk() output which did not end with '\n'.
    > >>
    > >> A caller is allowed to allocate/free "struct printk_buffer" using
    > >> kzalloc()/kfree() if that caller is in a location where it is possible
    > >> to do so.
    > >>
    > >> A macro named "DEFINE_PRINTK_BUFFER()" is defined for allocating
    > >> "struct printk_buffer" from the stack memory or in the .bss section.
    > >>
    > >> But since sizeof("struct printk_buffer") is nearly 1KB, it might not be
    > >> preferable to allocate "struct printk_buffer" from the stack memory.
    > >> In that case, a caller can use best-effort buffering mode. Two functions
    > >> get_printk_buffer() and put_printk_buffer() are provided for that mode.
    > >>
    > >> get_printk_buffer() tries to assign a "struct printk_buffer" from
    > >> statically preallocated array. It returns NULL if all static
    > >> "struct printk_buffer" are in use.
    > >>
    > >> put_printk_buffer() flushes and releases the "struct printk_buffer".
    > >> put_printk_buffer() must match corresponding get_printk_buffer() as with
    > >> rcu_read_unlock() must match corresponding rcu_read_lock().
    > >
    > > One problem with this API is when it is used in more complicated code
    > > and put_printk_buffer() is not called in some path. I mean leaking.
    > > We might get out of buffers easily.
    >
    > Then, as an debugging config option for statically preallocated buffers,
    > we could record how get_printk_buffer() was called, like lockdep records
    > where a lock was taken.

    Another solution might be to store some timestamp (jiffies?) into
    struct printk_buffer when a new message is added. Then we could flush
    stalled buffers in get_printk_buffer() with some warning.

    Unfortunately, it might be unsafe to put the stalled buffers.
    Well, it might be safe if there is a lock less access. I wonder
    if we could reuse the printk_safe code here.

    Anyway, I would like to have a solution before we add the new
    API into the kernel. We would need it sooner or later anyway.
    And I would like to be sure that the API is sane.


    > > A solution might be to store some information about the owner and
    > > put the buffer also when a non-buffered printk is called from
    > > the same context.
    > >
    > > It might even make it easier to use. If we are able to guess the
    > > buffer by the context, we do not need to pass it as an argument.
    >
    > It would be nice if we can omit passing "struct printk_buffer" argument.
    > But that results in "implicit contexts" which Linus has rejected
    > ( https://lkml.kernel.org/CA+55aFx+5R-vFQfr7+Ok9Yrs2adQ2Ma4fz+S6nCyWHY_-2mrmw@mail.gmail.com ).

    Yeah and the arguments for explicit context make sense when
    I reread them again.

    Best Regards,
    Petr

    \
     
     \ /
      Last update: 2018-10-09 16:53    [W:4.037 / U:0.516 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site