From: "Steven Rostedt (Google)" <[email protected]>
Limit the max print event of trace_marker to just 4K string size. This must
also be less than the amount that can be held by a trace_seq along with
the text that is before the output (like the task name, PID, CPU, state,
etc). As trace_seq is made to handle large events (some greater than 4K).
Make the max size of a trace_marker write event be 4K which is guaranteed
to fit in the trace_seq buffer.
Suggested-by: Mathieu Desnoyers <[email protected]>
Signed-off-by: Steven Rostedt (Google) <[email protected]>
---
Changes since v1: https://lore.kernel.org/linux-trace-kernel/[email protected]/
- Just make the max limit 4K and not half of the trace_seq size.
The trace_seq is already made to handle events greater than 4k.
kernel/trace/trace.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 8198bfc54b58..d16b95ca58a7 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -7293,6 +7293,8 @@ tracing_free_buffer_release(struct inode *inode, struct file *filp)
return 0;
}
+#define TRACE_MARKER_MAX_SIZE 4096
+
static ssize_t
tracing_mark_write(struct file *filp, const char __user *ubuf,
size_t cnt, loff_t *fpos)
@@ -7320,6 +7322,9 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,
if ((ssize_t)cnt < 0)
return -EINVAL;
+ if (cnt > TRACE_MARKER_MAX_SIZE)
+ cnt = TRACE_MARKER_MAX_SIZE;
+
meta_size = sizeof(*entry) + 2; /* add '\0' and possible '\n' */
again:
size = cnt + meta_size;
@@ -7328,11 +7333,6 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,
if (cnt < FAULTED_SIZE)
size += FAULTED_SIZE - cnt;
- if (size > TRACE_SEQ_BUFFER_SIZE) {
- cnt -= size - TRACE_SEQ_BUFFER_SIZE;
- goto again;
- }
-
buffer = tr->array_buffer.buffer;
event = __trace_buffer_lock_reserve(buffer, TRACE_PRINT, size,
tracing_gen_ctx());
--
2.43.0
On 2024-03-04 22:34, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <[email protected]>
>
> Limit the max print event of trace_marker to just 4K string size. This must
> also be less than the amount that can be held by a trace_seq along with
> the text that is before the output (like the task name, PID, CPU, state,
> etc). As trace_seq is made to handle large events (some greater than 4K).
> Make the max size of a trace_marker write event be 4K which is guaranteed
> to fit in the trace_seq buffer.
>
> Suggested-by: Mathieu Desnoyers <[email protected]>
From my perspective I only attempted to clarify the point Linus made
about limiting the trace_marker input to 4kB. Feel adapt the
Suggested-by tag accordingly.
Reviewed-by: Mathieu Desnoyers <[email protected]>
Thanks,
Mathieu
> Signed-off-by: Steven Rostedt (Google) <[email protected]>
> ---
> Changes since v1: https://lore.kernel.org/linux-trace-kernel/[email protected]/
>
> - Just make the max limit 4K and not half of the trace_seq size.
> The trace_seq is already made to handle events greater than 4k.
>
> kernel/trace/trace.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 8198bfc54b58..d16b95ca58a7 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -7293,6 +7293,8 @@ tracing_free_buffer_release(struct inode *inode, struct file *filp)
> return 0;
> }
>
> +#define TRACE_MARKER_MAX_SIZE 4096
> +
> static ssize_t
> tracing_mark_write(struct file *filp, const char __user *ubuf,
> size_t cnt, loff_t *fpos)
> @@ -7320,6 +7322,9 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,
> if ((ssize_t)cnt < 0)
> return -EINVAL;
>
> + if (cnt > TRACE_MARKER_MAX_SIZE)
> + cnt = TRACE_MARKER_MAX_SIZE;
> +
> meta_size = sizeof(*entry) + 2; /* add '\0' and possible '\n' */
> again:
> size = cnt + meta_size;
> @@ -7328,11 +7333,6 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,
> if (cnt < FAULTED_SIZE)
> size += FAULTED_SIZE - cnt;
>
> - if (size > TRACE_SEQ_BUFFER_SIZE) {
> - cnt -= size - TRACE_SEQ_BUFFER_SIZE;
> - goto again;
> - }
> -
> buffer = tr->array_buffer.buffer;
> event = __trace_buffer_lock_reserve(buffer, TRACE_PRINT, size,
> tracing_gen_ctx());
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
On Mon, 4 Mar 2024 22:34:33 -0500
Steven Rostedt <[email protected]> wrote:
> From: "Steven Rostedt (Google)" <[email protected]>
>
> Limit the max print event of trace_marker to just 4K string size. This must
> also be less than the amount that can be held by a trace_seq along with
> the text that is before the output (like the task name, PID, CPU, state,
> etc). As trace_seq is made to handle large events (some greater than 4K).
> Make the max size of a trace_marker write event be 4K which is guaranteed
> to fit in the trace_seq buffer.
>
> Suggested-by: Mathieu Desnoyers <[email protected]>
> Signed-off-by: Steven Rostedt (Google) <[email protected]>
This looks good to me.
Reviewed-by: Masami Hiramatsu (Google) <[email protected]>
Thank you,
> ---
> Changes since v1: https://lore.kernel.org/linux-trace-kernel/[email protected]/
>
> - Just make the max limit 4K and not half of the trace_seq size.
> The trace_seq is already made to handle events greater than 4k.
>
> kernel/trace/trace.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 8198bfc54b58..d16b95ca58a7 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -7293,6 +7293,8 @@ tracing_free_buffer_release(struct inode *inode, struct file *filp)
> return 0;
> }
>
> +#define TRACE_MARKER_MAX_SIZE 4096
> +
> static ssize_t
> tracing_mark_write(struct file *filp, const char __user *ubuf,
> size_t cnt, loff_t *fpos)
> @@ -7320,6 +7322,9 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,
> if ((ssize_t)cnt < 0)
> return -EINVAL;
>
> + if (cnt > TRACE_MARKER_MAX_SIZE)
> + cnt = TRACE_MARKER_MAX_SIZE;
> +
> meta_size = sizeof(*entry) + 2; /* add '\0' and possible '\n' */
> again:
> size = cnt + meta_size;
> @@ -7328,11 +7333,6 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,
> if (cnt < FAULTED_SIZE)
> size += FAULTED_SIZE - cnt;
>
> - if (size > TRACE_SEQ_BUFFER_SIZE) {
> - cnt -= size - TRACE_SEQ_BUFFER_SIZE;
> - goto again;
> - }
> -
> buffer = tr->array_buffer.buffer;
> event = __trace_buffer_lock_reserve(buffer, TRACE_PRINT, size,
> tracing_gen_ctx());
> --
> 2.43.0
>
--
Masami Hiramatsu (Google) <[email protected]>