[PATCH] tracing: Fix comment in tracing_buffers_splice_read()

Steven Rostedt posted 1 patch 3 weeks ago
kernel/trace/trace.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
[PATCH] tracing: Fix comment in tracing_buffers_splice_read()
Posted by Steven Rostedt 3 weeks ago
From: Steven Rostedt <rostedt@goodmis.org>

The comment about returning an error if the read fails on the first
iteration is slightly incorrect. It makes it sound like the only reason it
could fail on a later iteration is if the subbuf order changed. That is
incorrect, it could also fail if the length passed in was not a multiple
of the subbuf size. Fix the comment.

Link: https://lore.kernel.org/all/20260904143527.40e73d36@gandalf.local.home/
Fixes: TBD
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index b26c4c277ce5..8658cad53cb5 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -7296,11 +7296,13 @@ ssize_t tracing_buffers_splice_read(struct file *file, loff_t *ppos,
 			r = ring_buffer_read_page(ref->buffer, ref->rpage, len, iter->cpu_file, 1);
 		} else if (!i) {
 			/*
-			 * We failed to read because the length is too small
-			 * or unaligned. If this is the first iteration, it's
-			 * an invalid userspace input. Otherwise, this is due
-			 * to a subbuf order change. Do not report an error
-			 * and just finish the read.
+			 * If this fails to read on the first iteration, it
+			 * means the length was too small and an error should
+			 * be returned to user space. Otherwise, at least
+			 * one sub-buffer was successfully read but this failed
+			 * due to either the length was unaligned or the
+			 * subbuf order changed. Either case, do not report
+			 * an error.
 			 */
 			ret = -EINVAL;
 		}
-- 
2.53.0
Re: [PATCH] tracing: Fix comment in tracing_buffers_splice_read()
Posted by Vincent Donnefort 2 weeks, 6 days ago
On Fri, Sep 04, 2026 at 02:49:02PM -0400, Steven Rostedt wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
> 
> The comment about returning an error if the read fails on the first
> iteration is slightly incorrect. It makes it sound like the only reason it
> could fail on a later iteration is if the subbuf order changed. That is
> incorrect, it could also fail if the length passed in was not a multiple
> of the subbuf size. Fix the comment.
> 
> Link: https://lore.kernel.org/all/20260904143527.40e73d36@gandalf.local.home/
> Fixes: TBD
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
>  kernel/trace/trace.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index b26c4c277ce5..8658cad53cb5 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -7296,11 +7296,13 @@ ssize_t tracing_buffers_splice_read(struct file *file, loff_t *ppos,
>  			r = ring_buffer_read_page(ref->buffer, ref->rpage, len, iter->cpu_file, 1);
>  		} else if (!i) {
>  			/*
> -			 * We failed to read because the length is too small
> -			 * or unaligned. If this is the first iteration, it's
> -			 * an invalid userspace input. Otherwise, this is due
> -			 * to a subbuf order change. Do not report an error
> -			 * and just finish the read.
> +			 * If this fails to read on the first iteration, it
> +			 * means the length was too small and an error should
> +			 * be returned to user space. Otherwise, at least
> +			 * one sub-buffer was successfully read but this failed
> +			 * due to either the length was unaligned or the
> +			 * subbuf order changed. Either case, do not report
> +			 * an error.
>  			 */
>  			ret = -EINVAL;
>  		}
> -- 
> 2.53.0
> 

Ha yes, I see the missing case you were refering to now.

Reviewed-by: Vincent Donnefort <vdonnefort@google.com>

-- 
Vincent