Re: [PATCH] tracing: add trace_seq_reset function
From: Steven Rostedt <rostedt@goodmis.org>
Date: 2024-01-22 22:08:57
Also in:
lkml
On Mon, 22 Jan 2024 15:22:25 -0300 "Ricardo B. Marliere" [off-list ref] wrote:
Currently, trace_seq_init may be called many times with the intent of resetting the buffer. Add a function trace_seq_reset that does that and replace the relevant occurrences to use it instead.
Hi Ricardo! It's also OK to add to the commit log that the goal of this is to later extend trace_seq to be more flexible in the size of the buffer it holds. To do that, we need to differentiate between initializing a trace_seq and just resetting it.
quoted hunk ↗ jump to hunk
Signed-off-by: Ricardo B. Marliere <redacted> --- include/linux/trace_seq.h | 8 ++++++++ include/trace/trace_events.h | 2 +- kernel/trace/trace.c | 8 ++++---- kernel/trace/trace_seq.c | 2 +- 4 files changed, 14 insertions(+), 6 deletions(-)diff --git a/include/linux/trace_seq.h b/include/linux/trace_seq.h index 9ec229dfddaa..c28e0ccb50bd 100644 --- a/include/linux/trace_seq.h +++ b/include/linux/trace_seq.h@@ -29,6 +29,14 @@ trace_seq_init(struct trace_seq *s) s->readpos = 0; } +static inline void +trace_seq_reset(struct trace_seq *s) +{ + seq_buf_clear(&s->seq); + s->full = 0; + s->readpos = 0; +} + /** * trace_seq_used - amount of actual data written to buffer * @s: trace sequence descriptordiff --git a/include/trace/trace_events.h b/include/trace/trace_events.h index c2f9cabf154d..2bc79998e5ab 100644 --- a/include/trace/trace_events.h +++ b/include/trace/trace_events.h@@ -227,7 +227,7 @@ trace_raw_output_##call(struct trace_iterator *iter, int flags, \ \ field = (typeof(field))entry; \ \ - trace_seq_init(p); \ + trace_seq_reset(p); \ return trace_output_call(iter, #call, print); \
Note, p = &iter->tmp_seq
where it has never been initialized. To do this, we need to add:
trace_seq_init(&iter->tmp_seq);
where ever iter is created. You need to look at all the locations where
iter is created ("iter =") and differentiate where it is first used from
just passing pointers around.
The iter = kzalloc() will be easy, but for example, both seq and tmp_seq
need to be initialized in tracing_buffers_open().
Perhaps we need a:
if (WARN_ON_ONCE(!s->seq.size))
seq_buf_init(&s->seq, s->buffer, TRACE_SEQ_BUFFER_SIZE);
else
seq_buf_clear(&s->seq);
in the trace_seq_reset() to catch any place that doesn't have it
initialized yet.
-- Steve
quoted hunk ↗ jump to hunk
} \ static struct trace_event_functions trace_event_type_funcs_##call = { \diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index 46dbe22121e9..9147f3717b9a 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c@@ -6946,7 +6946,7 @@ tracing_read_pipe(struct file *filp, char __user *ubuf, /* reset all but tr, trace, and overruns */ trace_iterator_reset(iter); cpumask_clear(iter->started); - trace_seq_init(&iter->seq); + trace_seq_reset(&iter->seq); trace_event_read_lock(); trace_access_lock(iter->cpu_file);@@ -6993,7 +6993,7 @@ tracing_read_pipe(struct file *filp, char __user *ubuf, /* Now copy what we have to the user */ sret = trace_seq_to_user(&iter->seq, ubuf, cnt); if (iter->seq.readpos >= trace_seq_used(&iter->seq)) - trace_seq_init(&iter->seq); + trace_seq_reset(&iter->seq); /* * If there was nothing to send to user, in spite of consuming trace@@ -7125,7 +7125,7 @@ static ssize_t tracing_splice_read_pipe(struct file *filp, spd.partial[i].offset = 0; spd.partial[i].len = trace_seq_used(&iter->seq); - trace_seq_init(&iter->seq); + trace_seq_reset(&iter->seq); } trace_access_unlock(iter->cpu_file);@@ -10274,7 +10274,7 @@ trace_printk_seq(struct trace_seq *s) printk(KERN_TRACE "%s", s->buffer); - trace_seq_init(s); + trace_seq_reset(s); } void trace_init_global_iter(struct trace_iterator *iter)diff --git a/kernel/trace/trace_seq.c b/kernel/trace/trace_seq.c index c158d65a8a88..741b2f3d76c0 100644 --- a/kernel/trace/trace_seq.c +++ b/kernel/trace/trace_seq.c@@ -59,7 +59,7 @@ int trace_print_seq(struct seq_file *m, struct trace_seq *s) * do something else with the contents. */ if (!ret) - trace_seq_init(s); + trace_seq_reset(s); return ret; }