Re: [PATCH] ring-buffer: Add barrire in rb_move_tail

[lijiazi <jqqlijiazi@xxxxxxxxx>]

On Fri, Sep 02, 2022 at 08:56:41AM -0400, Steven Rostedt wrote:
> On Fri, 2 Sep 2022 11:47:58 +0800
> lijiazi <jqqlijiazi@xxxxxxxxx> wrote:
> 
> > Yes, I'm thinking this may be a race issue too.
> > But I didn't find race point through code review.
> 
> I think I may have found it. Can you test this patch?
> 
Sure, I will test this patch.
Thanks!
> [ Disclaimer, I did not even try to compile it ]
> 
> -- Steve
> 
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index d59b6a328b7f..0f47e68fee9b 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -4580,6 +4580,10 @@ rb_get_reader_page(struct ring_buffer_per_cpu *cpu_buffer)
>  	goto again;
>  
>   out:
> +	/* If the commit is past the end of page, a writer is still updating it */
> +	if (reader && rb_page_size(reader) > BUF_PAGE_SIZE)
> +		reader = NULL;
> +
>From ramdump, current reader page's commit is 0xff0, not bigger than
BUF_PAGE_SIZE:
crash> struct buffer_page 0xffffffd10b599580 -x
struct buffer_page {
  list = {
    next = 0xffffffd10b599500,
    prev = 0xffffffd10b599680
  },
  write = {
    a = {
      counter = 0x100ff0
    }
  },
  read = 0xfd4,
  entries = {
    a = {
      counter = 0x100053
    }
  },
  real_end = 0xfd4,
  page = 0xffffffd10b553000
}
crash> struct buffer_data_page 0xffffffd10b553000 -x
struct buffer_data_page {
  time_stamp = 0xe2679ca0cd3d,
  commit = {
    a = {
      counter = 0xff0
    }
  },
  data = 0xffffffd10b553010 "\b"
}
I also can extrace trace log from ramdump by crash-trace extension tool:
bsp: <...>-32191 [006] 249032.606401: signal_generate:      sig=17 errno=0 code=1 comm=WifiDiagnostics pid=1535 grp=1 res=1
bsp: <...>-32183 [006] 249032.625192: sched_process_exit:   comm=osi_bin pid=32183 prio=120
bsp: <...>-32196 [006] 249033.677333: sched_process_exit:   comm=ip pid=32196 prio=120
bsp: <...>-32196 [006] 249033.677562: signal_generate:      sig=17 errno=0 code=1 comm=sh pid=32195 grp=1 res=0
Above logs is on reader page, reader task try to read PADDING event
after this event and lead to crash.
below logs on tail page:
bsp: <...>-32195 [006] 249033.677596: signal_deliver:       sig=17 errno=0 code=1 sa_handler=6385258d68 sa_flags=0
bsp: <...>-32195 [006] 249033.678561: signal_deliver:       sig=17 errno=0 code=1 sa_handler=6385258d68 sa_flags=0
bsp: <...>-32195 [006] 249033.680041: sched_process_exit:   comm=sh pid=32195 prio=120
bsp: <...>-32195 [006] 249033.680096: signal_generate:      sig=17 errno=0 code=1 comm=osi_bin pid=4505 grp=1 res=2
>  	/* Update the read_stamp on the first event */
>  	if (reader && reader->read == 0)
>  		cpu_buffer->read_stamp = reader->page->time_stamp;