Re: [PATCH 07/11] tracing: trace_events_synth: Convert to printbuf

[Steven Rostedt]

On Mon, 15 Aug 2022 13:26:09 -0400
Kent Overstreet <kent.overstreet@xxxxxxxxx> wrote:

> From: Kent Overstreet <kent.overstreet@xxxxxxxxx>
> 
> This converts from seq_buf to printbuf.
> 
> This code was using seq_buf for building up dynamically allocated
> strings; the conversion uses printbuf's heap allocation functionality to
> simplify things (no longer need to calculate size of the output string).

As I stated before. I'll look into converting the seq_bufs to printbuf for
tracing at a later date. That includes this file.

Please remove this patch from the series.

> 
> Also, alphabetize the #includes.
> 

And although the #includes are not in the order the tracing directory
prefers, that order is upside-down x-mas tree, not alphabetical.

Thanks,

-- Steve


> Signed-off-by: Kent Overstreet <kent.overstreet@xxxxxxxxx>
> Cc: Steven Rostedt <rostedt@xxxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> ---
>  kernel/trace/trace_events_synth.c | 51 ++++++++++---------------------
>  1 file changed, 16 insertions(+), 35 deletions(-)
> 
> diff --git a/kernel/trace/trace_events_synth.c b/kernel/trace/trace_events_synth.c
> index 5e8c07aef0..720c75429c 100644
> --- a/kernel/trace/trace_events_synth.c
> +++ b/kernel/trace/trace_events_synth.c
> @@ -5,13 +5,14 @@
>   * Copyright (C) 2015, 2020 Tom Zanussi <tom.zanussi@xxxxxxxxxxxxxxx>
>   */
>  
> -#include <linux/module.h>
>  #include <linux/kallsyms.h>
> -#include <linux/security.h>
> +#include <linux/module.h>
>  #include <linux/mutex.h>
> +#include <linux/printbuf.h>
> +#include <linux/rculist.h>
> +#include <linux/security.h>
>  #include <linux/slab.h>
>  #include <linux/stacktrace.h>
> -#include <linux/rculist.h>
>  #include <linux/tracefs.h>
>  
>  /* for gfp flag names */
> @@ -611,7 +612,7 @@ static struct synth_field *parse_synth_field(int argc, char **argv,
>  	const char *prefix = NULL, *field_type = argv[0], *field_name, *array;
>  	struct synth_field *field;
>  	int len, ret = -ENOMEM;
> -	struct seq_buf s;
> +	struct printbuf buf;
>  	ssize_t size;
>  
>  	if (!strcmp(field_type, "unsigned")) {
> @@ -654,28 +655,16 @@ static struct synth_field *parse_synth_field(int argc, char **argv,
>  		goto free;
>  	}
>  
> -	len = strlen(field_type) + 1;
> -
> -	if (array)
> -		len += strlen(array);
> -
> -	if (prefix)
> -		len += strlen(prefix);
> -
> -	field->type = kzalloc(len, GFP_KERNEL);
> -	if (!field->type)
> -		goto free;
> -
> -	seq_buf_init(&s, field->type, len);
> +	buf = PRINTBUF;
>  	if (prefix)
> -		seq_buf_puts(&s, prefix);
> -	seq_buf_puts(&s, field_type);
> +		prt_str(&buf, prefix);
> +	prt_str(&buf, field_type);
>  	if (array)
> -		seq_buf_puts(&s, array);
> -	if (WARN_ON_ONCE(!seq_buf_buffer_left(&s)))
> +		prt_str(&buf, array);
> +	if (buf.allocation_failure)
>  		goto free;
>  
> -	s.buffer[s.len] = '\0';
> +	field->type = buf.buf;
>  
>  	size = synth_field_size(field->type);
>  	if (size < 0) {
> @@ -687,23 +676,15 @@ static struct synth_field *parse_synth_field(int argc, char **argv,
>  		goto free;
>  	} else if (size == 0) {
>  		if (synth_field_is_string(field->type)) {
> -			char *type;
> -
> -			len = sizeof("__data_loc ") + strlen(field->type) + 1;
> -			type = kzalloc(len, GFP_KERNEL);
> -			if (!type)
> -				goto free;
> -
> -			seq_buf_init(&s, type, len);
> -			seq_buf_puts(&s, "__data_loc ");
> -			seq_buf_puts(&s, field->type);
> +			buf = PRINTBUF;
> +			prt_str(&buf, "__data_loc ");
> +			prt_str(&buf, field->type);
>  
> -			if (WARN_ON_ONCE(!seq_buf_buffer_left(&s)))
> +			if (buf.allocation_failure)
>  				goto free;
> -			s.buffer[s.len] = '\0';
>  
>  			kfree(field->type);
> -			field->type = type;
> +			field->type = buf.buf;
>  
>  			field->is_dynamic = true;
>  			size = sizeof(u64);