[PATCH 0/2] ring-buffer: Fix forced 8-byte alignment event length

[PATCH 0/2] ring-buffer: Fix forced 8-byte alignment event length

[Hui Wang]

This series fixes the event length reported by ring_buffer_event_length()
when RB_FORCE_8BYTE_ALIGNMENT is enabled, and updates the ftrace
trace_marker_raw selftest to account for that layout.

On architectures where CONFIG_HAVE_64BIT_ALIGNED_ACCESS is enabled, the
ring buffer forces 8-byte alignment. In that mode, the event length is
stored in event->array[0] even for small data events, and the payload
starts from event->array[1]. However, ring_buffer_event_length() only
subtracted the extra length field for large events. As a result, small
events reported a payload length 4 bytes larger than expected.

This was observed on riscv64 with CONFIG_HAVE_64BIT_ALIGNED_ACCESS=y
when running the ftrace trace_marker_raw.tc selftest. The first patch
fixes the ring-buffer length calculation. The second patch updates the
selftest expectation when the running kernel uses forced 8-byte
alignment.

Hui Wang (2):
  ring-buffer: Fix event length with forced 8-byte alignment
  selftests/ftrace: Account for 8-byte aligned trace_marker_raw events

 kernel/trace/ring_buffer.c                    |  3 +-
 .../ftrace/test.d/00basic/trace_marker_raw.tc | 16 +++++++--
 .../testing/selftests/ftrace/test.d/functions | 33 +++++++++++++++++++
 3 files changed, 48 insertions(+), 4 deletions(-)

-- 
2.43.0

[PATCH 1/2] ring-buffer: Fix event length with forced 8-byte alignment

[Hui Wang]

When RB_FORCE_8BYTE_ALIGNMENT is true, rb_calculate_event_length()
reserves the space of event->array[0] for placing the data length and
rb_update_event() stores the data length in event->array[0]
accordingly. As a result the whole event length will add extra 4 bytes
for sizeof(event.array[0]) unconditionally.

But ring_buffer_event_length() only subtracts the
sizeof(event->array[0]) for events larger than RB_MAX_SMALL_DATA +
sizeof(event->array[0]). As a result, small events on architectures
with RB_FORCE_8BYTE_ALIGNMENT=true report a data length that is 4
bytes larger than expected.

To fix it, add the RB_FORCE_8BYTE_ALIGNMENT as a condition to subtract
the size of that length field whenever RB_FORCE_8BYTE_ALIGNMENT is
true.

This issue is observed in a riscv64 kernel with
CONFIG_HAVE_64BIT_ALIGNED_ACCESS set to y, when we run ftrace selftest
trace_marker_raw.tc, we get the weird log: for cases where the id is
1..100, the number of data field is 8*N, but once id exceeds 100, the
number of data field becomes 8*N+4:
 # 1 buf: 58 00 00 00 80 5e d1 63 (number of data field is 8*1)
 ...
 # a buf: 58 ...                  (number of data field is 8*2)
 ...
 # 64 buf: 58 ...                 (number of data field is 8*13)
 # 65 buf: 58 ...                 (number of data field is 8*13+4)

After applying this change, the number of data field keeps being 8*N+4
consistently.

Fixes: 2271048d1b3b ("ring-buffer: Do 8 byte alignment for 64 bit that can not handle 4 byte align")
Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
 kernel/trace/ring_buffer.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 56a328e94395..d9af2bbaf9c0 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -270,7 +270,8 @@ unsigned ring_buffer_event_length(struct ring_buffer_event *event)
 	if (event->type_len > RINGBUF_TYPE_DATA_TYPE_LEN_MAX)
 		return length;
 	length -= RB_EVNT_HDR_SIZE;
-	if (length > RB_MAX_SMALL_DATA + sizeof(event->array[0]))
+	if (length > RB_MAX_SMALL_DATA + sizeof(event->array[0]) ||
+	    RB_FORCE_8BYTE_ALIGNMENT)
                 length -= sizeof(event->array[0]);
 	return length;
 }
-- 
2.43.0

[PATCH 2/2] selftests/ftrace: Account for 8-byte aligned trace_marker_raw events

[Hui Wang]

trace_marker_raw.tc assumes that the raw marker payload length
reported in trace_pipe is the result of int((id + 3) / 4) * 4, but
that is not true on kernels with CONFIG_HAVE_64BIT_ALIGNED_ACCESS
enabled.

With forced 8-byte alignment, the ring buffer event forces 8-byte
alignment. The event length is stored in array[0], the payload data
and id are placed in a struct raw_data_entry which is stored starting
at array[1]. In this case, the printed payload data length is 8*N+4
bytes.

To make the testcase pass in this case, add a kconfig_enabled() helper
and use it to detect CONFIG_HAVE_64BIT_ALIGNED_ACCESS so
trace_marker_raw.tc can calculate the expected length correctly.

Assisted-by: Copilot:gpt-5.5
Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
 .../ftrace/test.d/00basic/trace_marker_raw.tc | 16 +++++++--
 .../testing/selftests/ftrace/test.d/functions | 33 +++++++++++++++++++
 2 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/ftrace/test.d/00basic/trace_marker_raw.tc b/tools/testing/selftests/ftrace/test.d/00basic/trace_marker_raw.tc
index 8e905d4fe6dd..beda0f8627b3 100644
--- a/tools/testing/selftests/ftrace/test.d/00basic/trace_marker_raw.tc
+++ b/tools/testing/selftests/ftrace/test.d/00basic/trace_marker_raw.tc
@@ -15,6 +15,11 @@ is_little_endian() {
 }
 
 little=`is_little_endian`
+raw_data_align=4
+
+if kconfig_enabled CONFIG_HAVE_64BIT_ALIGNED_ACCESS; then
+       raw_data_align=8
+fi
 
 make_str() {
 	id=$1
@@ -60,7 +65,8 @@ test_multiple_writes() {
 	echo stop > trace_marker
 
 	# Check to make sure the number of entries is the id (rounded up by 4)
-	awk '/.*: # [0-9a-f]* / {
+	# or is (((id + 3) rounded by 8) + 4) if raw_data_align is 8
+	awk -v data_align=$raw_data_align '/.*: # [0-9a-f]* / {
 			print;
 			cnt = -1;
 			for (i = 0; i < NF; i++) {
@@ -69,8 +75,12 @@ test_multiple_writes() {
 					i++;
 					cnt = strtonum("0x" $i);
 					num = NF - (i + 1);
-					# The number of items is always rounded up by 4
-					cnt2 = int((cnt + 3) / 4) * 4;
+					# The number of items is rounded up by 4
+					# or is (8 * N + 4) if data_align is 8
+					if (data_align == 4)
+						cnt2 = int((cnt + 3) / 4) * 4;
+					else
+						cnt2 = int((cnt + 3) / 8) * 8 + 4;
 					if (cnt2 != num) {
 						exit 1;
 					}
diff --git a/tools/testing/selftests/ftrace/test.d/functions b/tools/testing/selftests/ftrace/test.d/functions
index 826141e299e5..0f778087d81b 100644
--- a/tools/testing/selftests/ftrace/test.d/functions
+++ b/tools/testing/selftests/ftrace/test.d/functions
@@ -177,6 +177,39 @@ check_awk_strtonum() { # strtonum is GNU awk extension
     awk 'BEGIN{strtonum("0x1")}'
 }
 
+# a helper to check if a kconfig is enabled or not
+# return value: 0 (if kconfig is enabled)
+#               1 (if kconfig is not enabled)
+#               2 (if the config files don't exist or are unreadable)
+kconfig_enabled() { # config-name
+    local config="$1"
+    local uname_r=`uname -r`
+    local config_file
+
+    case "$config" in
+    CONFIG_*) ;;
+    *) config="CONFIG_$config" ;;
+    esac
+
+    if [ -f /proc/config.gz ] && zgrep --version >/dev/null 2>&1; then
+        zgrep -Eq "^${config}=(y|m)$" /proc/config.gz 2>/dev/null
+        return $?
+    fi
+
+    for config_file in \
+        /boot/config-$uname_r \
+        /lib/modules/$uname_r/config \
+        /lib/modules/$uname_r/build/.config
+    do
+        if [ -f "$config_file" ]; then
+            grep -Eq "^${config}=(y|m)$" "$config_file"
+            return $?
+        fi
+    done
+
+    return 2
+}
+
 LOCALHOST=127.0.0.1
 
 yield() {
-- 
2.43.0

Re: [PATCH 1/2] ring-buffer: Fix event length with forced 8-byte alignment

[Masami Hiramatsu]

On Sun,  7 Jun 2026 15:24:30 +0800
Hui Wang <hui.wang@canonical.com> wrote:

> When RB_FORCE_8BYTE_ALIGNMENT is true, rb_calculate_event_length()
> reserves the space of event->array[0] for placing the data length and
> rb_update_event() stores the data length in event->array[0]
> accordingly. As a result the whole event length will add extra 4 bytes
> for sizeof(event.array[0]) unconditionally.
> 
> But ring_buffer_event_length() only subtracts the
> sizeof(event->array[0]) for events larger than RB_MAX_SMALL_DATA +
> sizeof(event->array[0]). As a result, small events on architectures
> with RB_FORCE_8BYTE_ALIGNMENT=true report a data length that is 4
> bytes larger than expected.
> 
> To fix it, add the RB_FORCE_8BYTE_ALIGNMENT as a condition to subtract
> the size of that length field whenever RB_FORCE_8BYTE_ALIGNMENT is
> true.
> 
> This issue is observed in a riscv64 kernel with
> CONFIG_HAVE_64BIT_ALIGNED_ACCESS set to y, when we run ftrace selftest
> trace_marker_raw.tc, we get the weird log: for cases where the id is
> 1..100, the number of data field is 8*N, but once id exceeds 100, the
> number of data field becomes 8*N+4:
>  # 1 buf: 58 00 00 00 80 5e d1 63 (number of data field is 8*1)
>  ...
>  # a buf: 58 ...                  (number of data field is 8*2)
>  ...
>  # 64 buf: 58 ...                 (number of data field is 8*13)
>  # 65 buf: 58 ...                 (number of data field is 8*13+4)
> 
> After applying this change, the number of data field keeps being 8*N+4
> consistently.
> 

Good catch!

This looks good to me.

Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thanks,

> Fixes: 2271048d1b3b ("ring-buffer: Do 8 byte alignment for 64 bit that can not handle 4 byte align")
> Signed-off-by: Hui Wang <hui.wang@canonical.com>
> ---
>  kernel/trace/ring_buffer.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 56a328e94395..d9af2bbaf9c0 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -270,7 +270,8 @@ unsigned ring_buffer_event_length(struct ring_buffer_event *event)
>  	if (event->type_len > RINGBUF_TYPE_DATA_TYPE_LEN_MAX)
>  		return length;
>  	length -= RB_EVNT_HDR_SIZE;
> -	if (length > RB_MAX_SMALL_DATA + sizeof(event->array[0]))
> +	if (length > RB_MAX_SMALL_DATA + sizeof(event->array[0]) ||
> +	    RB_FORCE_8BYTE_ALIGNMENT)
>                  length -= sizeof(event->array[0]);
>  	return length;
>  }
> -- 
> 2.43.0
> 
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH 2/2] selftests/ftrace: Account for 8-byte aligned trace_marker_raw events

[Masami Hiramatsu]

On Sun,  7 Jun 2026 15:24:31 +0800
Hui Wang <hui.wang@canonical.com> wrote:

> trace_marker_raw.tc assumes that the raw marker payload length
> reported in trace_pipe is the result of int((id + 3) / 4) * 4, but
> that is not true on kernels with CONFIG_HAVE_64BIT_ALIGNED_ACCESS
> enabled.
> 
> With forced 8-byte alignment, the ring buffer event forces 8-byte
> alignment. The event length is stored in array[0], the payload data
> and id are placed in a struct raw_data_entry which is stored starting
> at array[1]. In this case, the printed payload data length is 8*N+4
> bytes.
> 
> To make the testcase pass in this case, add a kconfig_enabled() helper
> and use it to detect CONFIG_HAVE_64BIT_ALIGNED_ACCESS so
> trace_marker_raw.tc can calculate the expected length correctly.
> 

Hmm this fix lacks consideration for the environment.

> Assisted-by: Copilot:gpt-5.5
> Signed-off-by: Hui Wang <hui.wang@canonical.com>
> ---
>  .../ftrace/test.d/00basic/trace_marker_raw.tc | 16 +++++++--
>  .../testing/selftests/ftrace/test.d/functions | 33 +++++++++++++++++++
>  2 files changed, 46 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/ftrace/test.d/00basic/trace_marker_raw.tc b/tools/testing/selftests/ftrace/test.d/00basic/trace_marker_raw.tc
> index 8e905d4fe6dd..beda0f8627b3 100644
> --- a/tools/testing/selftests/ftrace/test.d/00basic/trace_marker_raw.tc
> +++ b/tools/testing/selftests/ftrace/test.d/00basic/trace_marker_raw.tc
> @@ -15,6 +15,11 @@ is_little_endian() {
>  }
>  
>  little=`is_little_endian`
> +raw_data_align=4
> +
> +if kconfig_enabled CONFIG_HAVE_64BIT_ALIGNED_ACCESS; then

Checking Kconfig is OK, but in this case, if the existence of the
dependent Kconfig file itself cannot be confirmed, this test should
return an unresolved error.

> +       raw_data_align=8
> +fi
>  
>  make_str() {
>  	id=$1
> @@ -60,7 +65,8 @@ test_multiple_writes() {
>  	echo stop > trace_marker
>  
>  	# Check to make sure the number of entries is the id (rounded up by 4)
> -	awk '/.*: # [0-9a-f]* / {
> +	# or is (((id + 3) rounded by 8) + 4) if raw_data_align is 8
> +	awk -v data_align=$raw_data_align '/.*: # [0-9a-f]* / {
>  			print;
>  			cnt = -1;
>  			for (i = 0; i < NF; i++) {
> @@ -69,8 +75,12 @@ test_multiple_writes() {
>  					i++;
>  					cnt = strtonum("0x" $i);
>  					num = NF - (i + 1);
> -					# The number of items is always rounded up by 4
> -					cnt2 = int((cnt + 3) / 4) * 4;
> +					# The number of items is rounded up by 4
> +					# or is (8 * N + 4) if data_align is 8
> +					if (data_align == 4)
> +						cnt2 = int((cnt + 3) / 4) * 4;
> +					else
> +						cnt2 = int((cnt + 3) / 8) * 8 + 4;
>  					if (cnt2 != num) {
>  						exit 1;
>  					}
> diff --git a/tools/testing/selftests/ftrace/test.d/functions b/tools/testing/selftests/ftrace/test.d/functions
> index 826141e299e5..0f778087d81b 100644
> --- a/tools/testing/selftests/ftrace/test.d/functions
> +++ b/tools/testing/selftests/ftrace/test.d/functions
> @@ -177,6 +177,39 @@ check_awk_strtonum() { # strtonum is GNU awk extension
>      awk 'BEGIN{strtonum("0x1")}'
>  }
>  
> +# a helper to check if a kconfig is enabled or not
> +# return value: 0 (if kconfig is enabled)
> +#               1 (if kconfig is not enabled)
> +#               2 (if the config files don't exist or are unreadable)
> +kconfig_enabled() { # config-name
> +    local config="$1"
> +    local uname_r=`uname -r`
> +    local config_file
> +
> +    case "$config" in
> +    CONFIG_*) ;;
> +    *) config="CONFIG_$config" ;;
> +    esac
> +
> +    if [ -f /proc/config.gz ] && zgrep --version >/dev/null 2>&1; then
> +        zgrep -Eq "^${config}=(y|m)$" /proc/config.gz 2>/dev/null

Do not use zgrep (this requires to install zgrep pacakge) in this test,
instead, use more widely available `gzip -dc | grep ...`.
I would like to keep this runnable on a minimum environment.

> +        return $?
> +    fi
> +
> +    for config_file in \
> +        /boot/config-$uname_r \
> +        /lib/modules/$uname_r/config \
> +        /lib/modules/$uname_r/build/.config

Hmm, also I don't like this, because this highly depends on the environment.
Instead, we can add CONFIG_IKCONFIG_PROC=y in tools/testing/selftests/ftrace/config.

Thank you,

> +    do
> +        if [ -f "$config_file" ]; then
> +            grep -Eq "^${config}=(y|m)$" "$config_file"
> +            return $?
> +        fi
> +    done
> +
> +    return 2
> +}
> +
>  LOCALHOST=127.0.0.1
>  
>  yield() {
> -- 
> 2.43.0
> 
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH 2/2] selftests/ftrace: Account for 8-byte aligned trace_marker_raw events

[Hui Wang]

On 6/8/26 17:17, Masami Hiramatsu (Google) wrote:
> On Sun,  7 Jun 2026 15:24:31 +0800
> Hui Wang <hui.wang@canonical.com> wrote:
>
[...]
> +    for config_file in \
> +        /boot/config-$uname_r \
> +        /lib/modules/$uname_r/config \
> +        /lib/modules/$uname_r/build/.config
>
> Hmm, also I don't like this, because this highly depends on the environment.
> Instead, we can add CONFIG_IKCONFIG_PROC=y in tools/testing/selftests/ftrace/config.
>
> Thank you,
>
Thanks for the review. I'll address all other comments in v2.

I have a concern about this specific point. On Ubuntu kernels, both 
CONFIG_IKCONFIG and CONFIG_IKCONFIG_PROC are disabled by default, so 
/proc/config.gz does not exist. If we drop the /boot/config-$(uname -r) 
lookup and rely solely on /proc/config.gz, this test would become 
unresolved on every Ubuntu kernel — a regression, since it works on 
those kernels today.

There is also existing precedent for the /boot/config-$(uname -r) 
fallback: tools/testing/selftests/mm/va_high_addr_switch.sh checks 
/proc/config.gz first and falls back to /boot/config-$(uname -r).

So how about we keep /boot/config-$(uname -r) as a fallback, but drop 
the /lib/modules/... paths you objected to. And add ftrace/config as you 
suggested here.

Thanks,
Hui.
>> +    do
>> +        if [ -f "$config_file" ]; then
>> +            grep -Eq "^${config}=(y|m)$" "$config_file"
>> +            return $?
>> +        fi
>> +    done
>> +
>> +    return 2
>> +}
>> +
>>   LOCALHOST=127.0.0.1
>>   
>>   yield() {
>> -- 
>> 2.43.0
>>
>>
>

Re: [PATCH 2/2] selftests/ftrace: Account for 8-byte aligned trace_marker_raw events

[Steven Rostedt]

On Sun,  7 Jun 2026 15:24:31 +0800
Hui Wang <hui.wang@canonical.com> wrote:

> trace_marker_raw.tc assumes that the raw marker payload length
> reported in trace_pipe is the result of int((id + 3) / 4) * 4, but
> that is not true on kernels with CONFIG_HAVE_64BIT_ALIGNED_ACCESS
> enabled.
> 
> With forced 8-byte alignment, the ring buffer event forces 8-byte
> alignment. The event length is stored in array[0], the payload data
> and id are placed in a struct raw_data_entry which is stored starting
> at array[1]. In this case, the printed payload data length is 8*N+4
> bytes.
> 
> To make the testcase pass in this case, add a kconfig_enabled() helper
> and use it to detect CONFIG_HAVE_64BIT_ALIGNED_ACCESS so
> trace_marker_raw.tc can calculate the expected length correctly.
> 
> Assisted-by: Copilot:gpt-5.5
> Signed-off-by: Hui Wang <hui.wang@canonical.com>

NACK

Let's not change the kernel for a broken test. Also this has already
been fixed but appears not to be applied yet.

Shuah, can you please apply the below fix.

  https://lore.kernel.org/all/20260601023251.1916483-1-dtcccc@linux.alibaba.com/

-- Steve

Re: [PATCH 1/2] ring-buffer: Fix event length with forced 8-byte alignment

[Steven Rostedt]

On Mon, 8 Jun 2026 18:02:45 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> On Sun,  7 Jun 2026 15:24:30 +0800
> Hui Wang <hui.wang@canonical.com> wrote:
> 
> > When RB_FORCE_8BYTE_ALIGNMENT is true, rb_calculate_event_length()
> > reserves the space of event->array[0] for placing the data length and
> > rb_update_event() stores the data length in event->array[0]
> > accordingly. As a result the whole event length will add extra 4 bytes
> > for sizeof(event.array[0]) unconditionally.
> > 
> > But ring_buffer_event_length() only subtracts the
> > sizeof(event->array[0]) for events larger than RB_MAX_SMALL_DATA +
> > sizeof(event->array[0]). As a result, small events on architectures
> > with RB_FORCE_8BYTE_ALIGNMENT=true report a data length that is 4
> > bytes larger than expected.
> > 
> > To fix it, add the RB_FORCE_8BYTE_ALIGNMENT as a condition to subtract
> > the size of that length field whenever RB_FORCE_8BYTE_ALIGNMENT is
> > true.
> > 
> > This issue is observed in a riscv64 kernel with
> > CONFIG_HAVE_64BIT_ALIGNED_ACCESS set to y, when we run ftrace selftest
> > trace_marker_raw.tc, we get the weird log: for cases where the id is
> > 1..100, the number of data field is 8*N, but once id exceeds 100, the
> > number of data field becomes 8*N+4:
> >  # 1 buf: 58 00 00 00 80 5e d1 63 (number of data field is 8*1)
> >  ...
> >  # a buf: 58 ...                  (number of data field is 8*2)
> >  ...
> >  # 64 buf: 58 ...                 (number of data field is 8*13)
> >  # 65 buf: 58 ...                 (number of data field is 8*13+4)
> > 
> > After applying this change, the number of data field keeps being 8*N+4
> > consistently.
> >   
> 
> Good catch!
> 
> This looks good to me.
> 
> Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

This is the patch I meant to reply to.

NACK as the test is broken and not the kernel.

There's a pending fix already:

  https://lore.kernel.org/all/20260601023251.1916483-1-dtcccc@linux.alibaba.com/


-- Steve