Re: [PATCH v2] ring-buffer: Annotate the speculative iterator event copy

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] ring-buffer: unannotated speculative reads in
  rb_iter_head_event()
--

commit b8f65c61949524a718551f053209be2d46d549d1
Author: Zhu Jun <zhujun2@cmss.chinamobile.com>

ring-buffer: Annotate the speculative iterator event copy

This commit wraps the speculative memcpy() in rb_iter_head_event() with
data_race() to suppress data race reports when a concurrent writer
overwrites the buffer page.

> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index e760d2fe386fb..6e51a7a75d6b1 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -3562,7 +3562,8 @@ rb_iter_head_event(struct ring_buffer_iter *iter)
>  		/* Writer corrupted the read? */
>  		goto reset;
>  
> -	memcpy(iter->event, event, length);
> +	/* The page stamp below detects a concurrent overwrite of this copy. */
> +	data_race(memcpy(iter->event, event, length));
>  	/*
>  	 * If the page stamp is still the same after this rmb() then the
>  	 * event was safely copied without the writer entering the page.

[Severity: Low]
Does annotating only the memcpy() leave the preceding rb_event_length(event)
and following iter_head_page->page->time_stamp loads in the same speculative
read section subject to the same data race?

Just above the memcpy(), rb_iter_head_event() calls rb_event_length(event)
on the same shared event pointer while a concurrent writer can update
event->type_len, event->time_delta and event->array[0] in rb_update_event():

kernel/trace/ring_buffer.c:rb_iter_head_event() {
	...
	event = __rb_page_index(iter_head_page, iter->head);
	length = rb_event_length(event);
	...
}

Because rb_event_length() and its helpers read the event header with plain
loads, the race detector will still report the race there, and the compiler
can reload the event->type_len bitfield across the switch and length
calculation before the barrier().

Similarly, the plain loads of iter_head_page->page->time_stamp after the
memcpy() race with __rb_reserve_next() updating tail_page->page->time_stamp.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113083446.177306-1-zhujun2@cmss.chinamobile.com?part=1