Hi, Steven

On (09/12/18 12:05), Steven Rostedt wrote:
> > : Introduce a few helper functions for it:
> > : 
> > :  init_line_buffer(&buf);
> > :  print_line(&buf, fmt, args);
> > :  vprint_line(&buf, fmt, vararg);
> > :  finish_line(&buf);
> > : 
> 
> This sounds like seq_buf to me.

Correct.

> > +struct pr_line {
> > +   char                    *buffer;
> > +   int                     size;
> > +   int                     len;
> > +   char                    *level;
> > +};
> 
> Can you look at implementing this with using a seq_buf?

Certainly, attached.

It doesn't seem to save us that much code, tho. It looks smaller just
because I dropped "truncated" print out and didn't include !CONFIG_PRINTK
noise this time around. And the OK thing about previous version was that
it didn't introduce any new dependencies to printk.

Making pr_line available via printk.h -- #include seq_buf.h in printk.h - at
glance looks like some fun. printk.h is getting included very early, before
we have all the stuff that seq_buf.h wants - we can remove fs.h from
seq_buf.h and add a bunch of forward declarations for path and seq_file;
but all those BUG_ON/WARN_ON/etc is another story (unless we want every
pr_line user to include seq_buf.h).

... maybe I can change API. But I sort of like that implicit buffer case:

        DEFINE_PR_LINE(KERN_ERR, pl);

        pr_line(&pl, "Hello, ");
        pr_line(&pl, "%s.\n", "Steven");

And, looking at potential users of pr_line, I'd say that we better
have DEFINE_PR_LINE_BUF, because some of them do print messages longer
than 80 chars.

===

From: Sergey Senozhatsky <sergey.senozhat...@gmail.com>
Subject: [PATCH] lib/seq_buf: add pr_line buffering API

Signed-off-by: Sergey Senozhatsky <sergey.senozhat...@gmail.com>
---
 include/linux/seq_buf.h | 35 +++++++++++++++++++++++++++++++
 lib/seq_buf.c           | 46 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 81 insertions(+)

diff --git a/include/linux/seq_buf.h b/include/linux/seq_buf.h
index aa5deb041c25..5e9a5ff9a440 100644
--- a/include/linux/seq_buf.h
+++ b/include/linux/seq_buf.h
@@ -23,6 +23,36 @@ struct seq_buf {
        loff_t                  readpos;
 };
 
+#define __SEQ_BUF_INITIALIZER(buf, length) {                           \
+       .buffer                 = (buf),                                \
+       .size                   = (length),                             \
+       .len                    = 0,                                    \
+       .readpos                = 0, }
+
+#ifdef CONFIG_PRINTK
+#define __PR_LINE_BUF_SZ       80
+#else
+#define __PR_LINE_BUF_SZ       0
+#endif
+
+struct pr_line {
+       struct seq_buf          sb;
+       char                    *level;
+};
+
+#define DEFINE_PR_LINE(lev, name)                                      \
+       char            __line[__PR_LINE_BUF_SZ];                       \
+       struct pr_line  name = {                                        \
+               .sb = __SEQ_BUF_INITIALIZER(__line, __PR_LINE_BUF_SZ),  \
+               .level  = lev,                                          \
+       }
+
+#define DEFINE_PR_LINE_BUF(lev, name, buf, sz)                         \
+       struct pr_line  name = {                                        \
+               .sb = __SEQ_BUF_INITIALIZER(buf, (sz)),         \
+               .level  = lev,                                          \
+       }
+
 static inline void seq_buf_clear(struct seq_buf *s)
 {
        s->len = 0;
@@ -131,4 +161,9 @@ 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, ...);
+extern void pr_line_flush(struct pr_line *pl);
 #endif /* _LINUX_SEQ_BUF_H */
diff --git a/lib/seq_buf.c b/lib/seq_buf.c
index 11f2ae0f9099..29bc4f24b83e 100644
--- a/lib/seq_buf.c
+++ b/lib/seq_buf.c
@@ -324,3 +324,49 @@ int seq_buf_to_user(struct seq_buf *s, char __user *ubuf, 
int cnt)
        s->readpos += cnt;
        return cnt;
 }
+
+int vpr_line(struct pr_line *pl, const char *fmt, va_list args)
+{
+       struct seq_buf *s = &pl->sb;
+       int ret, len;
+
+       if (fmt[0] == '\n') {
+               pr_line_flush(pl);
+               return 0;
+       }
+
+       ret = seq_buf_vprintf(s, fmt, args);
+
+       len = seq_buf_used(s);
+       if (len && s->buffer[len - 1] == '\n')
+               pr_line_flush(pl);
+
+       return ret;
+}
+EXPORT_SYMBOL(vpr_line);
+
+int pr_line(struct pr_line *pl, const char *fmt, ...)
+{
+       va_list ap;
+       int ret;
+
+       va_start(ap, fmt);
+       ret = vpr_line(pl, fmt, ap);
+       va_end(ap);
+
+       return ret;
+}
+EXPORT_SYMBOL(pr_line);
+
+void pr_line_flush(struct pr_line *pl)
+{
+       struct seq_buf *s = &pl->sb;
+       int len = seq_buf_used(s);
+
+       if (!len)
+               return;
+
+       printk("%s%.*s", pl->level, len, s->buffer);
+       seq_buf_clear(s);
+}
+EXPORT_SYMBOL(pr_line_flush);
-- 
2.19.0

Reply via email to