Messages in this thread | | | From | Uros Bizjak <> | Date | Sat, 4 Mar 2023 09:56:36 +0100 | Subject | Re: [PATCH v3 2/3] ring_buffer: Change some static functions to bool |
| |
On Fri, Mar 3, 2023 at 5:34 PM Mukesh Ojha <quic_mojha@quicinc.com> wrote:
> > -static int rb_time_cmpxchg(rb_time_t *t, u64 expect, u64 set) > > +static bool rb_time_cmpxchg(rb_time_t *t, u64 expect, u64 set) > > { > > unsigned long cnt, top, bottom, msb; > > unsigned long cnt2, top2, bottom2, msb2; > > @@ -1490,7 +1488,7 @@ rb_set_head_page(struct ring_buffer_per_cpu *cpu_buffer) > > return NULL; > > } > > > > -static int rb_head_page_replace(struct buffer_page *old, > > +static bool rb_head_page_replace(struct buffer_page *old, > > struct buffer_page *new) > > nit: did you miss ret in rb_get_reader_page()?
Indeed, thanks for noticing!
> > static __always_inline void > > @@ -5408,9 +5407,8 @@ bool ring_buffer_empty(struct trace_buffer *buffer) > > { > > struct ring_buffer_per_cpu *cpu_buffer; > > unsigned long flags; > > - bool dolock; > > + bool dolock, ret; > > int cpu; > > - int ret; > > nit: would have been nice if you would not have changed existing stuff > i.e each variable on a separate line. > > > > /* yes this is racy, but if you don't like the race, lock the buffer */ > > for_each_buffer_cpu(buffer, cpu) { > > @@ -5438,8 +5436,7 @@ bool ring_buffer_empty_cpu(struct trace_buffer *buffer, int cpu) > > { > > struct ring_buffer_per_cpu *cpu_buffer; > > unsigned long flags; > > - bool dolock; > > - int ret; > > + bool dolock, ret; > > same here
I'll do it in the way you propose in the follow-up revisiton.
> > > > if (!cpumask_test_cpu(cpu, buffer->cpumask)) > > return true; > > > Feel free to add > > Reviewed-by: Mukesh Ojha <quic_mojha@quicinc.com>
Thanks.
Uros.
| |