lkml.org 
[lkml]   [2018]   [Sep]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH] printk: inject caller information into the body of message
    From
    Date
    On 2018/09/14 15:57, Sergey Senozhatsky wrote:
    > On (09/13/18 23:28), Sergey Senozhatsky wrote:
    >> Not that I see any problems with pr_line_flush(). But can drop it, sure.
    >> pr_line() is a replacement for pr_cont() and as such it's not for multi-line
    >> buffering.
    >
    > OK, attached.
    > Let me know if anything needs to improved (including broken English).
    > Will we keep in the printk tree or shall I send a formal patch to Andrew?



    > @@ -20,6 +20,9 @@
    > * Annotation for a "continued" line of log printout (only done after a
    > * line that had no enclosing \n). Only to be used by core/arch code
    > * during early bootup (a continued line is not SMP-safe otherwise).
    > + *
    > + * Please consider pr_line()/vpr_line() functions for SMP-safe continued
    > + * line printing.

    I think the advantage is not limited to SMP-safeness. Reducing the frequency of
    calling printk() will reduce overhead. Also, latency for netconsole will be
    reduced by sending a whole line in one printk().



    > +/**
    > + * DEFINE_PR_LINE - define a new pr_line variable
    > + * @lev: printk() log level
    > + * @name: variable name
    > + *
    > + * Defines a new pr_line varialbe, which would use an implicit

    s/varialbe/variable/ .

    > + * stack buffer of size __PR_LINE_BUF_SZ.
    > + */
    > +#define DEFINE_PR_LINE(lev, name) \
    > + char __line_##name[__PR_LINE_BUF_SZ]; \
    > + struct pr_line name = { \
    > + .sb = __SEQ_BUF_INITIALIZER(__line_##name, \
    > + __PR_LINE_BUF_SZ), \
    > + .level = lev, \
    > + }

    Want a note that

    static DEFINE_PR_LINE(lev, name);

    won't make "name" variable "static" ?



    > +/**
    > + * DEFINE_PR_LINE_BUF - define a new pr_line variable
    > + * @lev: printk() log level
    > + * @name: variable name
    > + * @buf: external buffer
    > + * @sz: external buffer size
    > + *
    > + * Defines a new pr_line variable, which would use an external
    > + * buffer for printk line.
    > + */
    > +#define DEFINE_PR_LINE_BUF(lev, name, buf, sz) \
    > + struct pr_line name = { \
    > + .sb = __SEQ_BUF_INITIALIZER(buf, (sz)), \
    > + .level = lev, \
    > + }
    > +

    I would use this one for the OOM killer. 80 bytes is too short.

    static char oom_print_buf[1024];
    DEFINE_PR_LINE_BUF(level, oom_print_buf);



    > @@ -131,4 +187,8 @@ extern int
    > seq_buf_bprintf(struct seq_buf *s, const char *fmt, const u32 *binary);
    > #endif
    >
    > +extern __printf(2, 0)
    > +int vpr_line(struct pr_line *pl, const char *fmt, va_list args);
    > +extern __printf(2, 3)
    > +int pr_line(struct pr_line *pl, const char *fmt, ...);

    Do we want to mark "asmlinkage" like printk() ?

    > @@ -324,3 +324,60 @@ int seq_buf_to_user(struct seq_buf *s, char __user *ubuf, int cnt)
    > s->readpos += cnt;
    > return cnt;
    > }
    > +
    > +/**
    > + * vpr_line - Append data to the printk() line buffer
    > + * @pl: the pr_line descriptor

    s/descriptor/structure/ ?

    > + * @fmt: printf format string
    > + * @args: va_list of arguments from a printf() type function
    > + *
    > + * Writes a vnprintf() format into the printk() pr_line buffer.

    s/vnprintf/vprintf/ ?

    \
     
     \ /
      Last update: 2018-09-14 12:38    [W:3.193 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site