trace/ring_buffer: remove unused arguments

Message ID 1666274811-24138-1-git-send-email-chensong_2000@189.cn
State New
Headers
Series trace/ring_buffer: remove unused arguments |

Commit Message

Song Chen Oct. 20, 2022, 2:06 p.m. UTC
  As a argument, event has nothing to do with either
ring_buffer_unlock_commit or rb_commit and cleaning
it up has no side effect.

Signed-off-by: Song Chen <chensong_2000@189.cn>
---
 include/linux/ring_buffer.h          |  3 +--
 kernel/trace/ring_buffer.c           | 12 +++++-------
 kernel/trace/ring_buffer_benchmark.c |  2 +-
 kernel/trace/trace.c                 |  2 +-
 4 files changed, 8 insertions(+), 11 deletions(-)
  

Comments

Steven Rostedt Oct. 20, 2022, 2:15 p.m. UTC | #1
On Thu, 20 Oct 2022 22:06:51 +0800
Song Chen <chensong_2000@189.cn> wrote:

> As a argument, event has nothing to do with either
> ring_buffer_unlock_commit or rb_commit and cleaning
> it up has no side effect.

OK, so this is because of a389d86f7fd0 ("ring-buffer: Have nested events
still record running time stamp") which removed the usage of event in rb_commit():

 static void rb_commit(struct ring_buffer_per_cpu *cpu_buffer,
                      struct ring_buffer_event *event)
 {
        local_inc(&cpu_buffer->entries);
-       rb_update_write_stamp(cpu_buffer, event);
        rb_end_commit(cpu_buffer);
 }


I guess this doesn't hurt.

-- Steve

> 
> Signed-off-by: Song Chen <chensong_2000@189.cn>
> ---
>  include/linux/ring_buffer.h          |  3 +--
>  kernel/trace/ring_buffer.c           | 12 +++++-------
>  kernel/trace/ring_buffer_benchmark.c |  2 +-
>  kernel/trace/trace.c                 |  2 +-
>  4 files changed, 8 insertions(+), 11 deletions(-)
  
Song Chen Oct. 20, 2022, 3:25 p.m. UTC | #2
Hi,

On 2022/10/20 下午10:15, Steven Rostedt wrote:
> On Thu, 20 Oct 2022 22:06:51 +0800
> Song Chen <chensong_2000@189.cn> wrote:
> 
>> As a argument, event has nothing to do with either
>> ring_buffer_unlock_commit or rb_commit and cleaning
>> it up has no side effect.
> 
> OK, so this is because of a389d86f7fd0 ("ring-buffer: Have nested events
> still record running time stamp") which removed the usage of event in rb_commit():
> 
>   static void rb_commit(struct ring_buffer_per_cpu *cpu_buffer,
>                        struct ring_buffer_event *event)
>   {
>          local_inc(&cpu_buffer->entries);
> -       rb_update_write_stamp(cpu_buffer, event);
>          rb_end_commit(cpu_buffer);
>   }
> 
> 
> I guess this doesn't hurt.
> 
> -- Steve
> 

I was reading __kprobe_trace_func, store_trace_args stores args in 
fbuffer.entry,which is event->array, but at the end, rb_commit uses 
fbuffer.buffer instead of event.

Then i thought, if it's useless, why not just clean it up.

Song

>>
>> Signed-off-by: Song Chen <chensong_2000@189.cn>
>> ---
>>   include/linux/ring_buffer.h          |  3 +--
>>   kernel/trace/ring_buffer.c           | 12 +++++-------
>>   kernel/trace/ring_buffer_benchmark.c |  2 +-
>>   kernel/trace/trace.c                 |  2 +-
>>   4 files changed, 8 insertions(+), 11 deletions(-)
>
  
Steven Rostedt Oct. 20, 2022, 3:43 p.m. UTC | #3
On Thu, 20 Oct 2022 23:25:56 +0800
chensong <chensong_2000@189.cn> wrote:

> > I guess this doesn't hurt.
> 
> I was reading __kprobe_trace_func, store_trace_args stores args in 
> fbuffer.entry,which is event->array, but at the end, rb_commit uses 
> fbuffer.buffer instead of event.
> 
> Then i thought, if it's useless, why not just clean it up.


I meant your change probably doesn't hurt.

-- Steve
  

Patch

diff --git a/include/linux/ring_buffer.h b/include/linux/ring_buffer.h
index dac53fd3afea..a1483e5dd846 100644
--- a/include/linux/ring_buffer.h
+++ b/include/linux/ring_buffer.h
@@ -113,8 +113,7 @@  void ring_buffer_change_overwrite(struct trace_buffer *buffer, int val);
 
 struct ring_buffer_event *ring_buffer_lock_reserve(struct trace_buffer *buffer,
 						   unsigned long length);
-int ring_buffer_unlock_commit(struct trace_buffer *buffer,
-			      struct ring_buffer_event *event);
+int ring_buffer_unlock_commit(struct trace_buffer *buffer);
 int ring_buffer_write(struct trace_buffer *buffer,
 		      unsigned long length, void *data);
 
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index d59b6a328b7f..fbadfeff6542 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -3088,8 +3088,7 @@  static inline void rb_event_discard(struct ring_buffer_event *event)
 		event->time_delta = 1;
 }
 
-static void rb_commit(struct ring_buffer_per_cpu *cpu_buffer,
-		      struct ring_buffer_event *event)
+static void rb_commit(struct ring_buffer_per_cpu *cpu_buffer)
 {
 	local_inc(&cpu_buffer->entries);
 	rb_end_commit(cpu_buffer);
@@ -3298,15 +3297,14 @@  void ring_buffer_nest_end(struct trace_buffer *buffer)
  *
  * Must be paired with ring_buffer_lock_reserve.
  */
-int ring_buffer_unlock_commit(struct trace_buffer *buffer,
-			      struct ring_buffer_event *event)
+int ring_buffer_unlock_commit(struct trace_buffer *buffer)
 {
 	struct ring_buffer_per_cpu *cpu_buffer;
 	int cpu = raw_smp_processor_id();
 
 	cpu_buffer = buffer->buffers[cpu];
 
-	rb_commit(cpu_buffer, event);
+	rb_commit(cpu_buffer);
 
 	rb_wakeups(buffer, cpu_buffer);
 
@@ -3892,7 +3890,7 @@  int ring_buffer_write(struct trace_buffer *buffer,
 
 	memcpy(body, data, length);
 
-	rb_commit(cpu_buffer, event);
+	rb_commit(cpu_buffer);
 
 	rb_wakeups(buffer, cpu_buffer);
 
@@ -5877,7 +5875,7 @@  static __init int rb_write_something(struct rb_test_data *data, bool nested)
 	}
 
  out:
-	ring_buffer_unlock_commit(data->buffer, event);
+	ring_buffer_unlock_commit(data->buffer);
 
 	return 0;
 }
diff --git a/kernel/trace/ring_buffer_benchmark.c b/kernel/trace/ring_buffer_benchmark.c
index 78e576575b79..aef34673d79d 100644
--- a/kernel/trace/ring_buffer_benchmark.c
+++ b/kernel/trace/ring_buffer_benchmark.c
@@ -258,7 +258,7 @@  static void ring_buffer_producer(void)
 				hit++;
 				entry = ring_buffer_event_data(event);
 				*entry = smp_processor_id();
-				ring_buffer_unlock_commit(buffer, event);
+				ring_buffer_unlock_commit(buffer);
 			}
 		}
 		end_time = ktime_get();
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 31dd208a3d4e..d79d1a341f38 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -999,7 +999,7 @@  __buffer_unlock_commit(struct trace_buffer *buffer, struct ring_buffer_event *ev
 		/* ring_buffer_unlock_commit() enables preemption */
 		preempt_enable_notrace();
 	} else
-		ring_buffer_unlock_commit(buffer, event);
+		ring_buffer_unlock_commit(buffer);
 }
 
 /**