On 22/6/2569 BE 00:16, Jori Koolstra wrote:
>
>> Op 20-06-2026 19:54 CEST schreef Woradorn Laodhanadhaworn
>> <[email protected]>:
>>
>>
>> In preparation for removing the strlcat API[1],
>> replace the string concatenation logic with a struct seq_buf,
>> which tracks the current position and the remaining space internally.
>>
>> The backing buffer bootup_event_buf allocation is unchanged.
>> Use seq_buf_str() to NUL-terminate before passing to early_enable_events().
>>
>> Link: https://github.com/KSPP/linux/issues/370 [1]
>>
>> Signed-off-by: Woradorn Laodhanadhaworn <[email protected]>
>> ---
>> kernel/trace/trace_events.c | 21 ++++++++++++++++-----
>> 1 file changed, 16 insertions(+), 5 deletions(-)
>>
>> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
>> index c46e623e7e0d..15164723e028 100644
>> --- a/kernel/trace/trace_events.c
>> +++ b/kernel/trace/trace_events.c
>> @@ -22,6 +22,7 @@
>> #include <linux/sort.h>
>> #include <linux/slab.h>
>> #include <linux/delay.h>
>> +#include <linux/seq_buf.h>
>>
>> #include <trace/events/sched.h>
>> #include <trace/syscall.h>
>> @@ -4501,13 +4502,23 @@ extern struct trace_event_call
>> *__start_ftrace_events[];
>> extern struct trace_event_call *__stop_ftrace_events[];
>>
>> static char bootup_event_buf[COMMAND_LINE_SIZE] __initdata;
>
> Isn't this now unused?
>
>> +static struct seq_buf bootup_event_seq;
>> +static bool bootup_event_seq_initialized;
>>
>
> I think this can be refactored to avoid the bool. And should bootup_event_seq
> not be
> __initdata?
>
>> static __init int setup_trace_event(char *str)
>> {
>> - if (bootup_event_buf[0] != '\0')
>> - strlcat(bootup_event_buf, ",", COMMAND_LINE_SIZE);
>> + if (!bootup_event_seq_initialized) {
>> + seq_buf_init(&bootup_event_seq, bootup_event_buf,
>> COMMAND_LINE_SIZE);
>> + bootup_event_seq_initialized = true;
>> + }
>> +
>> + if (seq_buf_used(&bootup_event_seq) > 0)
>> + seq_buf_puts(&bootup_event_seq, ",");
>>
>> - strlcat(bootup_event_buf, str, COMMAND_LINE_SIZE);
>> + seq_buf_puts(&bootup_event_seq, str);
>> +
>> + if (seq_buf_has_overflowed(&bootup_event_seq))
>> + return -ENOMEM;
>>
>> trace_set_ring_buffer_expanded(NULL);
>> disable_tracing_selftest("running event tracing");
>> @@ -4766,7 +4777,7 @@ static __init int event_trace_enable(void)
>> */
>> __trace_early_add_events(tr);
>>
>> - early_enable_events(tr, bootup_event_buf, false);
>> + early_enable_events(tr, (char *)seq_buf_str(&bootup_event_seq), false);
>
> What if trace_event is empty? Then setup_trace_event does not run AFAIK. See
> the
> WARN_ON in seq_buf_str too. Have you tested this?
>
>>
>> trace_printk_start_comm();
>>
>> @@ -4794,7 +4805,7 @@ static __init int event_trace_enable_again(void)
>> if (!tr)
>> return -ENODEV;
>>
>> - early_enable_events(tr, bootup_event_buf, true);
>> + early_enable_events(tr, (char *)seq_buf_str(&bootup_event_seq), true);
>>
>> return 0;
>> }
>> --
>> 2.43.0
>
> Thanks,
> Jori.
Thank you, Jori, for your review. I will send v2.
I tested both empty and non-empty trace_event cases with QEMU, and both now
boot successfully.
The logs below demonstrate this.
Non-empty trace_event:
qemu-system-x86_64 \
-kernel arch/x86/boot/bzImage \
-initrd initramfs.cpio.gz \
-append "console=ttyS0
trace_event=:mod:rproc_qcom_common,:mod:qrtr,:mod:qcom_aoss
trace_event=:mod:rproc_qcom_common" \
-nographic \
-m 512M
[ 0.082316] Kernel command line: console=ttyS0
trace_event=:mod:rproc_qcom_common,:mod:qrtr,:mod:qcom_aoss
trace_event=:mod:rproc_qcom_common
[ 0.083324] bootup_event_buf: :mod:rproc_qcom_common,:mod:qrtr,:mod:qcom_aoss
[ 0.083413] bootup_event_buf:
:mod:rproc_qcom_common,:mod:qrtr,:mod:qcom_aoss,:mod:rproc_qcom_common
[ 0.083689] printk: log buffer data + meta data: 262144 + 917504 = 1179648
bytes
Empty trace_event:
qemu-system-x86_64 \
-kernel arch/x86/boot/bzImage \
-initrd initramfs.cpio.gz \
-append "console=ttyS0" \
-nographic \
-m 512M
[ 0.085213] Kernel command line: console=ttyS0
[ 0.086458] printk: log buffer data + meta data: 262144 + 917504 = 1179648
bytes
Thanks,
Woradorn
