[PATCH] tracing: move buffer in trace_seq to end of struct

[PATCH] tracing: move buffer in trace_seq to end of struct

[Elijah Wright]

TRACE_SEQ_BUFFER_SIZE is dependent on the architecture for its size. on 64-bit
systems, it is 8148 bytes. forced 8-byte alignment in size_t and seq_buf means
that trace_seq is 8200 bytes on 64-bit systems. moving the buffer to the end
of the struct fixes the issue. there shouldn't be any side effects, i.e.
pointer arithmetic on trace_seq

Signed-off-by: Elijah Wright <git@elijahs.space>
---
 include/linux/trace_seq.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/trace_seq.h b/include/linux/trace_seq.h
index a93ed5ac3226..557780fe1c77 100644
--- a/include/linux/trace_seq.h
+++ b/include/linux/trace_seq.h
@@ -21,10 +21,10 @@
 	(sizeof(struct seq_buf) + sizeof(size_t) + sizeof(int)))
 
 struct trace_seq {
-	char			buffer[TRACE_SEQ_BUFFER_SIZE];
 	struct seq_buf		seq;
 	size_t			readpos;
 	int			full;
+	char                    buffer[TRACE_SEQ_BUFFER_SIZE];
 };
 
 static inline void
-- 
2.43.0

Re: [PATCH] tracing: move buffer in trace_seq to end of struct

[Steven Rostedt]

On Wed, 20 Aug 2025 22:39:07 -0700
Elijah Wright <git@elijahs.space> wrote:

> TRACE_SEQ_BUFFER_SIZE is dependent on the architecture for its size. on 64-bit
> systems, it is 8148 bytes. forced 8-byte alignment in size_t and seq_buf means
> that trace_seq is 8200 bytes on 64-bit systems. moving the buffer to the end
> of the struct fixes the issue. there shouldn't be any side effects, i.e.
> pointer arithmetic on trace_seq

I don't remember why I had the buffer at the beginning. I think I did do
some crazy typecasting when I first developed this, as the buffer field
being first goes all the way back to the first commit (which doesn't
include prototypes before it).

I guess I'm fine with the change. Probably even makes the cache better, as
now the seq is now at the start of the buffer.

And that 'full' variable could actually be replaced as the seq does handle
those cases too. But it can't just be removed. I think the patch below
could be the answer to get rid of it.

Thanks,

-- Steve


diff --git a/include/linux/trace_seq.h b/include/linux/trace_seq.h
index a93ed5ac3226..92364deb39a5 100644
--- a/include/linux/trace_seq.h
+++ b/include/linux/trace_seq.h
@@ -24,14 +24,12 @@ struct trace_seq {
 	char			buffer[TRACE_SEQ_BUFFER_SIZE];
 	struct seq_buf		seq;
 	size_t			readpos;
-	int			full;
 };
 
 static inline void
 trace_seq_init(struct trace_seq *s)
 {
 	seq_buf_init(&s->seq, s->buffer, TRACE_SEQ_BUFFER_SIZE);
-	s->full = 0;
 	s->readpos = 0;
 }
 
@@ -77,7 +75,7 @@ trace_seq_buffer_ptr(struct trace_seq *s)
  */
 static inline bool trace_seq_has_overflowed(struct trace_seq *s)
 {
-	return s->full || seq_buf_has_overflowed(&s->seq);
+	return seq_buf_has_overflowed(&s->seq);
 }
 
 /*
diff --git a/kernel/trace/trace_seq.c b/kernel/trace/trace_seq.c
index c158d65a8a88..bcb4cf1307f8 100644
--- a/kernel/trace/trace_seq.c
+++ b/kernel/trace/trace_seq.c
@@ -24,9 +24,6 @@
 #include <linux/seq_file.h>
 #include <linux/trace_seq.h>
 
-/* How much buffer is left on the trace_seq? */
-#define TRACE_SEQ_BUF_LEFT(s) seq_buf_buffer_left(&(s)->seq)
-
 /*
  * trace_seq should work with being initialized with 0s.
  */
@@ -64,6 +61,37 @@ int trace_print_seq(struct seq_file *m, struct trace_seq *s)
 	return ret;
 }
 
+/* If buffer has overflowed, nul terminate the old end */
+static inline void reset_buf_cond(struct trace_seq *s, unsigned int end)
+{
+	if (likely(!seq_buf_has_overflowed(&s->seq)))
+		return;
+
+	/* If we can't write it all, don't bother writing anything */
+	if (end >= s->seq.size)
+		end = s->seq.size - 1;
+	s->buffer[end] = '\0';
+}
+
+static inline bool len_overflows_buf(struct trace_seq *s, unsigned int len)
+{
+	unsigned int end;
+
+	if (len < seq_buf_buffer_left(&(s)->seq))
+		return false;
+
+	/* Buffer is full, end it */
+	end = seq_buf_used(&s->seq);
+	if (end >= s->seq.size)
+		end = s->seq.size - 1;
+	s->buffer[end] = '\0';
+
+	/* Make the buffer overflowed */
+	seq_buf_set_overflow(&s->seq);
+
+	return true;
+}
+
 /**
  * trace_seq_printf - sequence printing of trace information
  * @s: trace sequence descriptor
@@ -80,20 +108,13 @@ void trace_seq_printf(struct trace_seq *s, const char *fmt, ...)
 	unsigned int save_len = s->seq.len;
 	va_list ap;
 
-	if (s->full)
-		return;
-
 	__trace_seq_init(s);
 
 	va_start(ap, fmt);
 	seq_buf_vprintf(&s->seq, fmt, ap);
 	va_end(ap);
 
-	/* If we can't write it all, don't bother writing anything */
-	if (unlikely(seq_buf_has_overflowed(&s->seq))) {
-		s->seq.len = save_len;
-		s->full = 1;
-	}
+	reset_buf_cond(s, save_len);
 }
 EXPORT_SYMBOL_GPL(trace_seq_printf);
 
@@ -110,17 +131,11 @@ void trace_seq_bitmask(struct trace_seq *s, const unsigned long *maskp,
 {
 	unsigned int save_len = s->seq.len;
 
-	if (s->full)
-		return;
-
 	__trace_seq_init(s);
 
 	seq_buf_printf(&s->seq, "%*pb", nmaskbits, maskp);
 
-	if (unlikely(seq_buf_has_overflowed(&s->seq))) {
-		s->seq.len = save_len;
-		s->full = 1;
-	}
+	reset_buf_cond(s, save_len);
 }
 EXPORT_SYMBOL_GPL(trace_seq_bitmask);
 
@@ -140,18 +155,11 @@ void trace_seq_vprintf(struct trace_seq *s, const char *fmt, va_list args)
 {
 	unsigned int save_len = s->seq.len;
 
-	if (s->full)
-		return;
-
 	__trace_seq_init(s);
 
 	seq_buf_vprintf(&s->seq, fmt, args);
 
-	/* If we can't write it all, don't bother writing anything */
-	if (unlikely(seq_buf_has_overflowed(&s->seq))) {
-		s->seq.len = save_len;
-		s->full = 1;
-	}
+	reset_buf_cond(s, save_len);
 }
 EXPORT_SYMBOL_GPL(trace_seq_vprintf);
 
@@ -174,19 +182,11 @@ void trace_seq_bprintf(struct trace_seq *s, const char *fmt, const u32 *binary)
 {
 	unsigned int save_len = s->seq.len;
 
-	if (s->full)
-		return;
-
 	__trace_seq_init(s);
 
 	seq_buf_bprintf(&s->seq, fmt, binary);
 
-	/* If we can't write it all, don't bother writing anything */
-	if (unlikely(seq_buf_has_overflowed(&s->seq))) {
-		s->seq.len = save_len;
-		s->full = 1;
-		return;
-	}
+	reset_buf_cond(s, save_len);
 }
 EXPORT_SYMBOL_GPL(trace_seq_bprintf);
 
@@ -204,15 +204,10 @@ void trace_seq_puts(struct trace_seq *s, const char *str)
 {
 	unsigned int len = strlen(str);
 
-	if (s->full)
-		return;
-
 	__trace_seq_init(s);
 
-	if (len > TRACE_SEQ_BUF_LEFT(s)) {
-		s->full = 1;
+	if (len_overflows_buf(s, len))
 		return;
-	}
 
 	seq_buf_putmem(&s->seq, str, len);
 }
@@ -230,15 +225,10 @@ EXPORT_SYMBOL_GPL(trace_seq_puts);
  */
 void trace_seq_putc(struct trace_seq *s, unsigned char c)
 {
-	if (s->full)
-		return;
-
 	__trace_seq_init(s);
 
-	if (TRACE_SEQ_BUF_LEFT(s) < 1) {
-		s->full = 1;
+	if (len_overflows_buf(s, len))
 		return;
-	}
 
 	seq_buf_putc(&s->seq, c);
 }
@@ -256,15 +246,10 @@ EXPORT_SYMBOL_GPL(trace_seq_putc);
  */
 void trace_seq_putmem(struct trace_seq *s, const void *mem, unsigned int len)
 {
-	if (s->full)
-		return;
-
 	__trace_seq_init(s);
 
-	if (len > TRACE_SEQ_BUF_LEFT(s)) {
-		s->full = 1;
+	if (len_overflows_buf(s, len))
 		return;
-	}
 
 	seq_buf_putmem(&s->seq, mem, len);
 }
@@ -285,16 +270,11 @@ void trace_seq_putmem_hex(struct trace_seq *s, const void *mem,
 {
 	unsigned int save_len = s->seq.len;
 
-	if (s->full)
-		return;
-
 	__trace_seq_init(s);
 
 	/* Each byte is represented by two chars */
-	if (len * 2 > TRACE_SEQ_BUF_LEFT(s)) {
-		s->full = 1;
+	if (len_overflows_buf(s, len * 2))
 		return;
-	}
 
 	/* The added spaces can still cause an overflow */
 	seq_buf_putmem_hex(&s->seq, mem, len);
@@ -328,10 +308,8 @@ int trace_seq_path(struct trace_seq *s, const struct path *path)
 
 	__trace_seq_init(s);
 
-	if (TRACE_SEQ_BUF_LEFT(s) < 1) {
-		s->full = 1;
-		return 0;
-	}
+	if (len_overflows_buf(s, 1))
+		return;
 
 	seq_buf_path(&s->seq, path, "\n");
 
@@ -387,10 +365,8 @@ int trace_seq_hex_dump(struct trace_seq *s, const char *prefix_str,
 
 	__trace_seq_init(s);
 
-	if (TRACE_SEQ_BUF_LEFT(s) < 1) {
-		s->full = 1;
-		return 0;
-	}
+	if (len_overflows_buf(s, 1))
+		return;
 
 	seq_buf_hex_dump(&(s->seq), prefix_str,
 		   prefix_type, rowsize, groupsize,

Re: [PATCH] tracing: move buffer in trace_seq to end of struct

[Steven Rostedt]

On Thu, 21 Aug 2025 11:43:55 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> diff --git a/include/linux/trace_seq.h b/include/linux/trace_seq.h
> index a93ed5ac3226..92364deb39a5 100644
> --- a/include/linux/trace_seq.h
> +++ b/include/linux/trace_seq.h
> @@ -24,14 +24,12 @@ struct trace_seq {
>  	char			buffer[TRACE_SEQ_BUFFER_SIZE];
>  	struct seq_buf		seq;
>  	size_t			readpos;
> -	int			full;
>  };
>  

I should have tried compiling it before posting. But trace.c has this:

		ret = print_trace_line(iter);
		if (ret == TRACE_TYPE_PARTIAL_LINE) {
			iter->seq.full = 0;
			trace_seq_puts(&iter->seq, "[LINE TOO BIG]\n");
		}

I need to figure out a clean way to fix that too :-p

-- Steve

Re: [PATCH] tracing: move buffer in trace_seq to end of struct

[Elijah]

can we maybe encode the overflow state in seq_buf? check if 
seq_buf_has_overflowed and clamp len back to the used size 
(seq_buf_used) in a helper?

On 8/21/2025 8:53 AM, Steven Rostedt wrote:
> On Thu, 21 Aug 2025 11:43:55 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
>> diff --git a/include/linux/trace_seq.h b/include/linux/trace_seq.h
>> index a93ed5ac3226..92364deb39a5 100644
>> --- a/include/linux/trace_seq.h
>> +++ b/include/linux/trace_seq.h
>> @@ -24,14 +24,12 @@ struct trace_seq {
>>   	char			buffer[TRACE_SEQ_BUFFER_SIZE];
>>   	struct seq_buf		seq;
>>   	size_t			readpos;
>> -	int			full;
>>   };
>>   
> 
> I should have tried compiling it before posting. But trace.c has this:
> 
> 		ret = print_trace_line(iter);
> 		if (ret == TRACE_TYPE_PARTIAL_LINE) {
> 			iter->seq.full = 0;
> 			trace_seq_puts(&iter->seq, "[LINE TOO BIG]\n");
> 		}
> 
> I need to figure out a clean way to fix that too :-p
> 
> -- Steve

Re: [PATCH] tracing: move buffer in trace_seq to end of struct

[Steven Rostedt]

On Thu, 21 Aug 2025 11:32:17 -0700
Elijah <me@elijahs.space> wrote:

> can we maybe encode the overflow state in seq_buf? check if 
> seq_buf_has_overflowed and clamp len back to the used size 
> (seq_buf_used) in a helper?

I could add a bit to the size?

diff --git a/include/linux/seq_buf.h b/include/linux/seq_buf.h
index 52791e070506..ea4996851901 100644
--- a/include/linux/seq_buf.h
+++ b/include/linux/seq_buf.h
@@ -20,8 +20,9 @@
  */
 struct seq_buf {
 	char			*buffer;
-	size_t			size;
-	size_t			len;
+	unsigned int		size;
+	unsigned int		len:31;
+	unsigned int		full:1;
 };
 
 #define DECLARE_SEQ_BUF(NAME, SIZE)			\


-- Steve

Re: [PATCH] tracing: move buffer in trace_seq to end of struct

[Elijah]

that could work yeah

On 9/2/2025 1:00 PM, Steven Rostedt wrote:
>   struct seq_buf {
>   	char			*buffer;
> -	size_t			size;
> -	size_t			len;
> +	unsigned int		size;
> +	unsigned int		len:31;
> +	unsigned int		full:1;
>   };