On Tue, 27 May 2025 14:52:16 -0400 Steven Rostedt <rost...@goodmis.org> wrote:
> From: Steven Rostedt <rost...@goodmis.org> > > The function ring_buffer_read_page() had two gotos. One was simply > returning "ret" and the other was unlocking the reader_lock. > > There's no reason to use goto to simply return the "ret" variable. Instead > just return the value. > > The jump to the unlocking of the reader_lock can be replaced by > guard(raw_spinlock_irqsave)(&cpu_buffer->reader_lock). > > With these two changes the "ret" variable is no longer used and can be > removed. The return value on non-error is what was read and is stored in > the "read" variable. > Looks good to me. Reviewed-by: Masami Hiramatsu (Google) <mhira...@kernel.org> Thanks! > Signed-off-by: Steven Rostedt (Google) <rost...@goodmis.org> > --- > kernel/trace/ring_buffer.c | 28 +++++++++++----------------- > 1 file changed, 11 insertions(+), 17 deletions(-) > > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c > index 0b21d64e90c8..897ce51d3bbf 100644 > --- a/kernel/trace/ring_buffer.c > +++ b/kernel/trace/ring_buffer.c > @@ -6535,38 +6535,37 @@ int ring_buffer_read_page(struct trace_buffer *buffer, > struct buffer_data_page *bpage; > struct buffer_page *reader; > unsigned long missed_events; > - unsigned long flags; > unsigned int commit; > unsigned int read; > u64 save_timestamp; > - int ret = -1; > > if (!cpumask_test_cpu(cpu, buffer->cpumask)) > - goto out; > + return -1; > > /* > * If len is not big enough to hold the page header, then > * we can not copy anything. > */ > if (len <= BUF_PAGE_HDR_SIZE) > - goto out; > + return -1; > > len -= BUF_PAGE_HDR_SIZE; > > if (!data_page || !data_page->data) > - goto out; > + return -1; > + > if (data_page->order != buffer->subbuf_order) > - goto out; > + return -1; > > bpage = data_page->data; > if (!bpage) > - goto out; > + return -1; > > - raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags); > + guard(raw_spinlock_irqsave)(&cpu_buffer->reader_lock); > > reader = rb_get_reader_page(cpu_buffer); > if (!reader) > - goto out_unlock; > + return -1; > > event = rb_reader_event(cpu_buffer); > > @@ -6600,7 +6599,7 @@ int ring_buffer_read_page(struct trace_buffer *buffer, > if (full && > (!read || (len < (commit - read)) || > cpu_buffer->reader_page == cpu_buffer->commit_page)) > - goto out_unlock; > + return -1; > > if (len > (commit - read)) > len = (commit - read); > @@ -6609,7 +6608,7 @@ int ring_buffer_read_page(struct trace_buffer *buffer, > size = rb_event_ts_length(event); > > if (len < size) > - goto out_unlock; > + return -1; > > /* save the current timestamp, since the user will need it */ > save_timestamp = cpu_buffer->read_stamp; > @@ -6667,7 +6666,6 @@ int ring_buffer_read_page(struct trace_buffer *buffer, > if (reader->real_end) > local_set(&bpage->commit, reader->real_end); > } > - ret = read; > > cpu_buffer->lost_events = 0; > > @@ -6694,11 +6692,7 @@ int ring_buffer_read_page(struct trace_buffer *buffer, > if (commit < buffer->subbuf_size) > memset(&bpage->data[commit], 0, buffer->subbuf_size - commit); > > - out_unlock: > - raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags); > - > - out: > - return ret; > + return read; > } > EXPORT_SYMBOL_GPL(ring_buffer_read_page); > > -- > 2.47.2 > -- Masami Hiramatsu (Google) <mhira...@kernel.org>
