Messages in this thread | | | Subject | Re: [PATCH] printk: inject caller information into the body of message | From | Tetsuo Handa <> | Date | Fri, 14 Sep 2018 19:37:21 +0900 |
| |
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/ ?
| |