[PATCHv2] seq_buf: add seq_buf_do_printk() helper

[PATCHv2] seq_buf: add seq_buf_do_printk() helper

[Sergey Senozhatsky]

Sometimes we use seq_buf to format a string buffer, which
we then pass to printk(). However, in certain situations
the seq_buf string buffer can get too big, exceeding the
PRINTKRB_RECORD_MAX bytes limit, and causing printk() to
truncate the string.

Add a new seq_buf helper. This helper prints the seq_buf
string buffer line by line, using \n as a delimiter,
rather than passing the whole string buffer to printk()
at once.

Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 include/linux/seq_buf.h |  2 ++
 lib/seq_buf.c           | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/include/linux/seq_buf.h b/include/linux/seq_buf.h
index 5b31c5147969..515d7fcb9634 100644
--- a/include/linux/seq_buf.h
+++ b/include/linux/seq_buf.h
@@ -159,4 +159,6 @@ extern int
 seq_buf_bprintf(struct seq_buf *s, const char *fmt, const u32 *binary);
 #endif
 
+void seq_buf_do_printk(struct seq_buf *s, const char *lvl);
+
 #endif /* _LINUX_SEQ_BUF_H */
diff --git a/lib/seq_buf.c b/lib/seq_buf.c
index 0a68f7aa85d6..be7227d42b20 100644
--- a/lib/seq_buf.c
+++ b/lib/seq_buf.c
@@ -93,6 +93,40 @@ int seq_buf_printf(struct seq_buf *s, const char *fmt, ...)
 }
 EXPORT_SYMBOL_GPL(seq_buf_printf);
 
+/**
+ * seq_buf_do_printk - printk seq_buf line by line
+ * @s: seq_buf descriptor
+ * @lvl: printk level
+ *
+ * printk()-s a multi-line sequential buffer line by line. The function
+ * makes sure that the buffer in @s is nul terminated and safe to read
+ * as a string.
+ */
+void seq_buf_do_printk(struct seq_buf *s, const char *lvl)
+{
+	const char *start, *lf;
+	int len;
+
+	if (s->size == 0 || s->len == 0)
+		return;
+
+	seq_buf_terminate(s);
+
+	start = s->buffer;
+	while ((lf = strchr(start, '\n'))) {
+		len = lf - start + 1;
+		printk("%s%.*s", lvl, len, start);
+		start = ++lf;
+	}
+
+	/* No trailing LF */
+	if (start < s->buffer + s->len) {
+		len = s->buffer + s->len - start;
+		printk("%s%.*s\n", lvl, len, start);
+	}
+}
+EXPORT_SYMBOL_GPL(seq_buf_do_printk);
+
 #ifdef CONFIG_BINARY_PRINTF
 /**
  * seq_buf_bprintf - Write the printf string from binary arguments
-- 
2.40.0.577.gac1e443424-goog

Re: [PATCHv2] seq_buf: add seq_buf_do_printk() helper

[Steven Rostedt]

On Tue, 11 Apr 2023 23:38:52 +0900
Sergey Senozhatsky <senozhatsky@chromium.org> wrote:

> Sometimes we use seq_buf to format a string buffer, which
> we then pass to printk(). However, in certain situations
> the seq_buf string buffer can get too big, exceeding the
> PRINTKRB_RECORD_MAX bytes limit, and causing printk() to
> truncate the string.
> 
> Add a new seq_buf helper. This helper prints the seq_buf
> string buffer line by line, using \n as a delimiter,
> rather than passing the whole string buffer to printk()
> at once.
> 
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>

Looks good to me. Want me to pull this in my tree, or do you have
patches dependent on this?

-- Steve

Re: [PATCHv2] seq_buf: add seq_buf_do_printk() helper

[Sergey Senozhatsky]

On (23/04/11 22:46), Steven Rostedt wrote:
> > Sometimes we use seq_buf to format a string buffer, which
> > we then pass to printk(). However, in certain situations
> > the seq_buf string buffer can get too big, exceeding the
> > PRINTKRB_RECORD_MAX bytes limit, and causing printk() to
> > truncate the string.
> > 
> > Add a new seq_buf helper. This helper prints the seq_buf
> > string buffer line by line, using \n as a delimiter,
> > rather than passing the whole string buffer to printk()
> > at once.
> > 
> > Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> 
> Looks good to me. Want me to pull this in my tree, or do you have
> patches dependent on this?

Thanks, would be great if you can pull this in your tree. The MM
patches that Yosry is working on will be posted after the merge
window.

Re: [PATCHv2] seq_buf: add seq_buf_do_printk() helper

[Yosry Ahmed]

On Tue, Apr 11, 2023 at 8:08 PM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> On (23/04/11 22:46), Steven Rostedt wrote:
> > > Sometimes we use seq_buf to format a string buffer, which
> > > we then pass to printk(). However, in certain situations
> > > the seq_buf string buffer can get too big, exceeding the
> > > PRINTKRB_RECORD_MAX bytes limit, and causing printk() to
> > > truncate the string.
> > >
> > > Add a new seq_buf helper. This helper prints the seq_buf
> > > string buffer line by line, using \n as a delimiter,
> > > rather than passing the whole string buffer to printk()
> > > at once.
> > >
> > > Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> >
> > Looks good to me. Want me to pull this in my tree, or do you have
> > patches dependent on this?
>
> Thanks, would be great if you can pull this in your tree. The MM
> patches that Yosry is working on will be posted after the merge
> window.

Yes, thanks Sergey and Steven.

The sooner this patch ends up in Linus's tree the better for me so I
can send my MM patches based off of it. I am guessing through your
tree is the easiest way for that.

Thanks again.

Re: [PATCHv2] seq_buf: add seq_buf_do_printk() helper

[Petr Mladek]

On Tue 2023-04-11 23:38:52, Sergey Senozhatsky wrote:
> Sometimes we use seq_buf to format a string buffer, which
> we then pass to printk(). However, in certain situations
> the seq_buf string buffer can get too big, exceeding the
> PRINTKRB_RECORD_MAX bytes limit, and causing printk() to
> truncate the string.
> 
> Add a new seq_buf helper. This helper prints the seq_buf
> string buffer line by line, using \n as a delimiter,
> rather than passing the whole string buffer to printk()
> at once.
> 
> --- a/lib/seq_buf.c
> +++ b/lib/seq_buf.c
> @@ -93,6 +93,40 @@ int seq_buf_printf(struct seq_buf *s, const char *fmt, ...)
>  }
>  EXPORT_SYMBOL_GPL(seq_buf_printf);
>  
> +/**
> + * seq_buf_do_printk - printk seq_buf line by line
> + * @s: seq_buf descriptor
> + * @lvl: printk level
> + *
> + * printk()-s a multi-line sequential buffer line by line. The function
> + * makes sure that the buffer in @s is nul terminated and safe to read
> + * as a string.
> + */
> +void seq_buf_do_printk(struct seq_buf *s, const char *lvl)
> +{
> +	const char *start, *lf;
> +	int len;
> +
> +	if (s->size == 0 || s->len == 0)
> +		return;
> +
> +	seq_buf_terminate(s);
> +
> +	start = s->buffer;
> +	while ((lf = strchr(start, '\n'))) {
> +		len = lf - start + 1;
> +		printk("%s%.*s", lvl, len, start);
> +		start = ++lf;
> +	}
> +
> +	/* No trailing LF */
> +	if (start < s->buffer + s->len) {
> +		len = s->buffer + s->len - start;
> +		printk("%s%.*s\n", lvl, len, start);

We know that the string is '\0' terminated, so the last print
might be easier:

	if (start < s->buffer + s->len)
		printk("%s%s\n", lvl, start);

Anyway, it looks good. With or without this change:

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

Re: [PATCHv2] seq_buf: add seq_buf_do_printk() helper

[Sergey Senozhatsky]

On (23/04/14 11:00), Petr Mladek wrote:
> > +void seq_buf_do_printk(struct seq_buf *s, const char *lvl)
> > +{
> > +	const char *start, *lf;
> > +	int len;
> > +
> > +	if (s->size == 0 || s->len == 0)
> > +		return;
> > +
> > +	seq_buf_terminate(s);
> > +
> > +	start = s->buffer;
> > +	while ((lf = strchr(start, '\n'))) {
> > +		len = lf - start + 1;
> > +		printk("%s%.*s", lvl, len, start);
> > +		start = ++lf;
> > +	}
> > +
> > +	/* No trailing LF */
> > +	if (start < s->buffer + s->len) {
> > +		len = s->buffer + s->len - start;
> > +		printk("%s%.*s\n", lvl, len, start);
> 
> We know that the string is '\0' terminated, so the last print
> might be easier:
> 
> 	if (start < s->buffer + s->len)
> 		printk("%s%s\n", lvl, start);

Indeed. Steven, let me know if you'd prefer a v3.

> Anyway, it looks good. With or without this change:
> 
> Reviewed-by: Petr Mladek <pmladek@suse.com>

Thanks Petr!

Re: [PATCHv2] seq_buf: add seq_buf_do_printk() helper

[Steven Rostedt]

On April 15, 2023 3:04:06 AM EDT, Sergey Senozhatsky <senozhatsky@chromium.org> wrote:
>On (23/04/14 11:00), Petr Mladek wrote:
>> > +void seq_buf_do_printk(struct seq_buf *s, const char *lvl)
>> > +{
>> > +	const char *start, *lf;
>> > +	int len;
>> > +
>> > +	if (s->size == 0 || s->len == 0)
>> > +		return;
>> > +
>> > +	seq_buf_terminate(s);
>> > +
>> > +	start = s->buffer;
>> > +	while ((lf = strchr(start, '\n'))) {
>> > +		len = lf - start + 1;
>> > +		printk("%s%.*s", lvl, len, start);
>> > +		start = ++lf;
>> > +	}
>> > +
>> > +	/* No trailing LF */
>> > +	if (start < s->buffer + s->len) {
>> > +		len = s->buffer + s->len - start;
>> > +		printk("%s%.*s\n", lvl, len, start);
>> 
>> We know that the string is '\0' terminated, so the last print
>> might be easier:
>> 
>> 	if (start < s->buffer + s->len)
>> 		printk("%s%s\n", lvl, start);
>
>Indeed. Steven, let me know if you'd prefer a v3.

Sure. Why not.

-- Steve

>
>> Anyway, it looks good. With or without this change:
>> 
>> Reviewed-by: Petr Mladek <pmladek@suse.com>
>
>Thanks Petr!

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity and top posting.