All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] ring-buffer: Fix forced 8-byte alignment event length
@ 2026-06-07  7:24 Hui Wang
  2026-06-07  7:24 ` [PATCH 1/2] ring-buffer: Fix event length with forced 8-byte alignment Hui Wang
  2026-06-07  7:24 ` [PATCH 2/2] selftests/ftrace: Account for 8-byte aligned trace_marker_raw events Hui Wang
  0 siblings, 2 replies; 10+ messages in thread
From: Hui Wang @ 2026-06-07  7:24 UTC (permalink / raw)
  To: rostedt, mhiramat, mathieu.desnoyers, pjw, linux-trace-kernel,
	shuah, wangfushuai, linux-kselftest
  Cc: 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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/2] ring-buffer: Fix event length with forced 8-byte alignment
  2026-06-07  7:24 [PATCH 0/2] ring-buffer: Fix forced 8-byte alignment event length Hui Wang
@ 2026-06-07  7:24 ` Hui Wang
  2026-06-08  9:02   ` Masami Hiramatsu
  2026-06-07  7:24 ` [PATCH 2/2] selftests/ftrace: Account for 8-byte aligned trace_marker_raw events Hui Wang
  1 sibling, 1 reply; 10+ messages in thread
From: Hui Wang @ 2026-06-07  7:24 UTC (permalink / raw)
  To: rostedt, mhiramat, mathieu.desnoyers, pjw, linux-trace-kernel,
	shuah, wangfushuai, linux-kselftest
  Cc: 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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] selftests/ftrace: Account for 8-byte aligned trace_marker_raw events
  2026-06-07  7:24 [PATCH 0/2] ring-buffer: Fix forced 8-byte alignment event length Hui Wang
  2026-06-07  7:24 ` [PATCH 1/2] ring-buffer: Fix event length with forced 8-byte alignment Hui Wang
@ 2026-06-07  7:24 ` Hui Wang
  2026-06-08  9:17   ` Masami Hiramatsu
  2026-06-08 16:50   ` Steven Rostedt
  1 sibling, 2 replies; 10+ messages in thread
From: Hui Wang @ 2026-06-07  7:24 UTC (permalink / raw)
  To: rostedt, mhiramat, mathieu.desnoyers, pjw, linux-trace-kernel,
	shuah, wangfushuai, linux-kselftest
  Cc: 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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] ring-buffer: Fix event length with forced 8-byte alignment
  2026-06-07  7:24 ` [PATCH 1/2] ring-buffer: Fix event length with forced 8-byte alignment Hui Wang
@ 2026-06-08  9:02   ` Masami Hiramatsu
  2026-06-08 16:52     ` Steven Rostedt
  0 siblings, 1 reply; 10+ messages in thread
From: Masami Hiramatsu @ 2026-06-08  9:02 UTC (permalink / raw)
  To: Hui Wang
  Cc: rostedt, mathieu.desnoyers, pjw, linux-trace-kernel, shuah,
	wangfushuai, linux-kselftest

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>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] selftests/ftrace: Account for 8-byte aligned trace_marker_raw events
  2026-06-07  7:24 ` [PATCH 2/2] selftests/ftrace: Account for 8-byte aligned trace_marker_raw events Hui Wang
@ 2026-06-08  9:17   ` Masami Hiramatsu
  2026-06-08 14:51     ` Hui Wang
  2026-06-08 16:50   ` Steven Rostedt
  1 sibling, 1 reply; 10+ messages in thread
From: Masami Hiramatsu @ 2026-06-08  9:17 UTC (permalink / raw)
  To: Hui Wang
  Cc: rostedt, mathieu.desnoyers, pjw, linux-trace-kernel, shuah,
	wangfushuai, linux-kselftest

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>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] selftests/ftrace: Account for 8-byte aligned trace_marker_raw events
  2026-06-08  9:17   ` Masami Hiramatsu
@ 2026-06-08 14:51     ` Hui Wang
  0 siblings, 0 replies; 10+ messages in thread
From: Hui Wang @ 2026-06-08 14:51 UTC (permalink / raw)
  To: Masami Hiramatsu (Google)
  Cc: rostedt, mathieu.desnoyers, pjw, linux-trace-kernel, shuah,
	wangfushuai, linux-kselftest


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
>>
>>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] selftests/ftrace: Account for 8-byte aligned trace_marker_raw events
  2026-06-07  7:24 ` [PATCH 2/2] selftests/ftrace: Account for 8-byte aligned trace_marker_raw events Hui Wang
  2026-06-08  9:17   ` Masami Hiramatsu
@ 2026-06-08 16:50   ` Steven Rostedt
  1 sibling, 0 replies; 10+ messages in thread
From: Steven Rostedt @ 2026-06-08 16:50 UTC (permalink / raw)
  To: Hui Wang
  Cc: mhiramat, mathieu.desnoyers, pjw, linux-trace-kernel, shuah,
	wangfushuai, linux-kselftest

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] ring-buffer: Fix event length with forced 8-byte alignment
  2026-06-08  9:02   ` Masami Hiramatsu
@ 2026-06-08 16:52     ` Steven Rostedt
  2026-06-09  4:22       ` Hui Wang
  0 siblings, 1 reply; 10+ messages in thread
From: Steven Rostedt @ 2026-06-08 16:52 UTC (permalink / raw)
  To: Masami Hiramatsu (Google)
  Cc: Hui Wang, mathieu.desnoyers, pjw, linux-trace-kernel, shuah,
	wangfushuai, linux-kselftest

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] ring-buffer: Fix event length with forced 8-byte alignment
  2026-06-08 16:52     ` Steven Rostedt
@ 2026-06-09  4:22       ` Hui Wang
  2026-06-10 16:17         ` Steven Rostedt
  0 siblings, 1 reply; 10+ messages in thread
From: Hui Wang @ 2026-06-09  4:22 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu (Google)
  Cc: mathieu.desnoyers, pjw, linux-trace-kernel, shuah, wangfushuai,
	linux-kselftest


On 6/9/26 00:52, Steven Rostedt wrote:
> 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

Hi Steven,

Thanks for the pointer. I reverted my two patches and applied the patch 
you referenced, but unfortunately it doesn't resolve the problem — the 
testcase still fails in my environment (riscv64 kernel with 
CONFIG_HAVE_64BIT_ALIGNED_ACCESS enabled).

 From what I can tell, that fix addresses a different problem than the 
one I'm hitting: it targets a 64K page-size issue, whereas my failure is 
caused by the 64-bit alignment requirement 
(CONFIG_HAVE_64BIT_ALIGNED_ACCESS). So I don't think they're the same 
root cause.

So can you please take a look at them again.

Thanks,

Hui.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] ring-buffer: Fix event length with forced 8-byte alignment
  2026-06-09  4:22       ` Hui Wang
@ 2026-06-10 16:17         ` Steven Rostedt
  0 siblings, 0 replies; 10+ messages in thread
From: Steven Rostedt @ 2026-06-10 16:17 UTC (permalink / raw)
  To: Hui Wang
  Cc: Masami Hiramatsu (Google), mathieu.desnoyers, pjw,
	linux-trace-kernel, shuah, wangfushuai, linux-kselftest

On Tue, 9 Jun 2026 12:22:47 +0800
Hui Wang <hui.wang@canonical.com> wrote:

> Thanks for the pointer. I reverted my two patches and applied the patch 
> you referenced, but unfortunately it doesn't resolve the problem — the 
> testcase still fails in my environment (riscv64 kernel with 
> CONFIG_HAVE_64BIT_ALIGNED_ACCESS enabled).
> 
>  From what I can tell, that fix addresses a different problem than the 
> one I'm hitting: it targets a 64K page-size issue, whereas my failure is 
> caused by the 64-bit alignment requirement 
> (CONFIG_HAVE_64BIT_ALIGNED_ACCESS). So I don't think they're the same 
> root cause.
> 
> So can you please take a look at them again.

OK, taking a deeper look at it, and yes, your are correct. Sorry for
jumping to the conclusion with thinking this was the same issue as what
was brought up before.

I'll take these.

Thanks,

-- Steve

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-06-10 16:17 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-07  7:24 [PATCH 0/2] ring-buffer: Fix forced 8-byte alignment event length Hui Wang
2026-06-07  7:24 ` [PATCH 1/2] ring-buffer: Fix event length with forced 8-byte alignment Hui Wang
2026-06-08  9:02   ` Masami Hiramatsu
2026-06-08 16:52     ` Steven Rostedt
2026-06-09  4:22       ` Hui Wang
2026-06-10 16:17         ` Steven Rostedt
2026-06-07  7:24 ` [PATCH 2/2] selftests/ftrace: Account for 8-byte aligned trace_marker_raw events Hui Wang
2026-06-08  9:17   ` Masami Hiramatsu
2026-06-08 14:51     ` Hui Wang
2026-06-08 16:50   ` Steven Rostedt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.