[PATCH 0/5] tracing: Various updates

[PATCH 0/5] tracing: Various updates

[Steven Rostedt]

 - Make trace_event_buffer_lock_reserver always disable preemption
   (it says it does but not always)

 - Have kprobes, uprobes and eprobes use
   trace_event_buffer_lock_reserver() functions.

Steven Rostedt (VMware) (5):
      tracing: Use __this_cpu_read() in trace_event_buffer_lock_reserver()
      tracing: Disable preemption when using the filter buffer
      tracing: Have eprobes use filtering logic of trace events
      tracing/kprobes: Do not open code event reserve logic
      tracing/uprobes: Use trace_event_buffer_reserve() helper

----
 kernel/trace/trace.c        | 59 +++++++++++++++++++++++++--------------------
 kernel/trace/trace.h        |  4 ++-
 kernel/trace/trace_eprobe.c | 16 ++++--------
 kernel/trace/trace_kprobe.c | 25 ++++++-------------
 kernel/trace/trace_uprobe.c | 10 +++-----
 5 files changed, 52 insertions(+), 62 deletions(-)

[PATCH 1/5] tracing: Use __this_cpu_read() in trace_event_buffer_lock_reserver()

[Steven Rostedt]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

The value read by this_cpu_read() is used later and its use is expected to
stay on the same CPU as being read. But this_cpu_read() does not warn if
it is called without preemption disabled, where as __this_cpu_read() will
check if preemption is disabled on CONFIG_DEBUG_PREEMPT

Currently all callers have preemption disabled, but there may be new
callers in the future that may not.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 4821fe6a40a5..2e87b7bf2ba7 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2746,7 +2746,7 @@ trace_event_buffer_lock_reserve(struct trace_buffer **current_rb,
 
 	if (!tr->no_filter_buffering_ref &&
 	    (trace_file->flags & (EVENT_FILE_FL_SOFT_DISABLED | EVENT_FILE_FL_FILTERED)) &&
-	    (entry = this_cpu_read(trace_buffered_event))) {
+	    (entry = __this_cpu_read(trace_buffered_event))) {
 		/*
 		 * Filtering is on, so try to use the per cpu buffer first.
 		 * This buffer will simulate a ring_buffer_event,
-- 
2.33.0

[PATCH 2/5] tracing: Disable preemption when using the filter buffer

[Steven Rostedt]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

In case trace_event_buffer_lock_reserve() is called with preemption
enabled, the algorithm that defines the usage of the per cpu filter buffer
may fail if the task schedules to another CPU after determining which
buffer it will use.

Disable preemption when using the filter buffer. And because that same
buffer must be used throughout the call, keep preemption disabled until
the filter buffer is released.

This will also keep the semantics between the use case of when the filter
buffer is used, and when the ring buffer itself is used, as that case also
disables preemption until the ring buffer is released.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace.c | 59 +++++++++++++++++++++++++-------------------
 kernel/trace/trace.h |  4 ++-
 2 files changed, 36 insertions(+), 27 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 2e87b7bf2ba7..415f00d70b15 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -980,6 +980,8 @@ __buffer_unlock_commit(struct trace_buffer *buffer, struct ring_buffer_event *ev
 		ring_buffer_write(buffer, event->array[0], &event->array[1]);
 		/* Release the temp buffer */
 		this_cpu_dec(trace_buffered_event_cnt);
+		/* ring_buffer_unlock_commit() enables preemption */
+		preempt_enable_notrace();
 	} else
 		ring_buffer_unlock_commit(buffer, event);
 }
@@ -2745,8 +2747,8 @@ trace_event_buffer_lock_reserve(struct trace_buffer **current_rb,
 	*current_rb = tr->array_buffer.buffer;
 
 	if (!tr->no_filter_buffering_ref &&
-	    (trace_file->flags & (EVENT_FILE_FL_SOFT_DISABLED | EVENT_FILE_FL_FILTERED)) &&
-	    (entry = __this_cpu_read(trace_buffered_event))) {
+	    (trace_file->flags & (EVENT_FILE_FL_SOFT_DISABLED | EVENT_FILE_FL_FILTERED))) {
+		preempt_disable_notrace();
 		/*
 		 * Filtering is on, so try to use the per cpu buffer first.
 		 * This buffer will simulate a ring_buffer_event,
@@ -2764,33 +2766,38 @@ trace_event_buffer_lock_reserve(struct trace_buffer **current_rb,
 		 * is still quicker than no copy on match, but having
 		 * to discard out of the ring buffer on a failed match.
 		 */
-		int max_len = PAGE_SIZE - struct_size(entry, array, 1);
+		if (entry = __this_cpu_read(trace_buffered_event)) {
+			int max_len = PAGE_SIZE - struct_size(entry, array, 1);
 
-		val = this_cpu_inc_return(trace_buffered_event_cnt);
+			val = this_cpu_inc_return(trace_buffered_event_cnt);
 
-		/*
-		 * Preemption is disabled, but interrupts and NMIs
-		 * can still come in now. If that happens after
-		 * the above increment, then it will have to go
-		 * back to the old method of allocating the event
-		 * on the ring buffer, and if the filter fails, it
-		 * will have to call ring_buffer_discard_commit()
-		 * to remove it.
-		 *
-		 * Need to also check the unlikely case that the
-		 * length is bigger than the temp buffer size.
-		 * If that happens, then the reserve is pretty much
-		 * guaranteed to fail, as the ring buffer currently
-		 * only allows events less than a page. But that may
-		 * change in the future, so let the ring buffer reserve
-		 * handle the failure in that case.
-		 */
-		if (val == 1 && likely(len <= max_len)) {
-			trace_event_setup(entry, type, trace_ctx);
-			entry->array[0] = len;
-			return entry;
+			/*
+			 * Preemption is disabled, but interrupts and NMIs
+			 * can still come in now. If that happens after
+			 * the above increment, then it will have to go
+			 * back to the old method of allocating the event
+			 * on the ring buffer, and if the filter fails, it
+			 * will have to call ring_buffer_discard_commit()
+			 * to remove it.
+			 *
+			 * Need to also check the unlikely case that the
+			 * length is bigger than the temp buffer size.
+			 * If that happens, then the reserve is pretty much
+			 * guaranteed to fail, as the ring buffer currently
+			 * only allows events less than a page. But that may
+			 * change in the future, so let the ring buffer reserve
+			 * handle the failure in that case.
+			 */
+			if (val == 1 && likely(len <= max_len)) {
+				trace_event_setup(entry, type, trace_ctx);
+				entry->array[0] = len;
+				/* Return with preemption disabled */
+				return entry;
+			}
+			this_cpu_dec(trace_buffered_event_cnt);
 		}
-		this_cpu_dec(trace_buffered_event_cnt);
+		/* __trace_buffer_lock_reserve() disables preemption */
+		preempt_enable_notrace();
 	}
 
 	entry = __trace_buffer_lock_reserve(*current_rb, type, len,
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 7162157b970b..8bd1a815ce90 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1337,10 +1337,12 @@ __trace_event_discard_commit(struct trace_buffer *buffer,
 			     struct ring_buffer_event *event)
 {
 	if (this_cpu_read(trace_buffered_event) == event) {
-		/* Simply release the temp buffer */
+		/* Simply release the temp buffer and enable preemption */
 		this_cpu_dec(trace_buffered_event_cnt);
+		preempt_enable_notrace();
 		return;
 	}
+	/* ring_buffer_discard_commit() enables preemption */
 	ring_buffer_discard_commit(buffer, event);
 }
 
-- 
2.33.0

Re: [PATCH 2/5] tracing: Disable preemption when using the filter buffer

[kernel test robot]

Hi Steven,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on rostedt-trace/for-next]
[also build test WARNING on linux/master hnaz-mm/master linus/master v5.16-rc3 next-20211129]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Steven-Rostedt/tracing-Various-updates/20211130-104342
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git for-next
config: mips-allyesconfig (https://download.01.org/0day-ci/archive/20211130/202111301209.GarWNR3z-lkp@intel.com/config)
compiler: mips-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/1ac91c8764ae50601cd41dceb620205607ab59f6
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Steven-Rostedt/tracing-Various-updates/20211130-104342
        git checkout 1ac91c8764ae50601cd41dceb620205607ab59f6
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=mips SHELL=/bin/bash kernel/trace/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   kernel/trace/trace.c: In function 'trace_event_buffer_lock_reserve':
>> kernel/trace/trace.c:2769:21: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
    2769 |                 if (entry = __this_cpu_read(trace_buffered_event)) {
         |                     ^~~~~
   kernel/trace/trace.c: In function 'trace_check_vprintf':
   kernel/trace/trace.c:3820:17: warning: function 'trace_check_vprintf' might be a candidate for 'gnu_printf' format attribute [-Wsuggest-attribute=format]
    3820 |                 trace_seq_vprintf(&iter->seq, iter->fmt, ap);
         |                 ^~~~~~~~~~~~~~~~~
   kernel/trace/trace.c:3887:17: warning: function 'trace_check_vprintf' might be a candidate for 'gnu_printf' format attribute [-Wsuggest-attribute=format]
    3887 |                 trace_seq_vprintf(&iter->seq, p, ap);
         |                 ^~~~~~~~~~~~~~~~~


vim +2769 kernel/trace/trace.c

  2736	
  2737	struct ring_buffer_event *
  2738	trace_event_buffer_lock_reserve(struct trace_buffer **current_rb,
  2739				  struct trace_event_file *trace_file,
  2740				  int type, unsigned long len,
  2741				  unsigned int trace_ctx)
  2742	{
  2743		struct ring_buffer_event *entry;
  2744		struct trace_array *tr = trace_file->tr;
  2745		int val;
  2746	
  2747		*current_rb = tr->array_buffer.buffer;
  2748	
  2749		if (!tr->no_filter_buffering_ref &&
  2750		    (trace_file->flags & (EVENT_FILE_FL_SOFT_DISABLED | EVENT_FILE_FL_FILTERED))) {
  2751			preempt_disable_notrace();
  2752			/*
  2753			 * Filtering is on, so try to use the per cpu buffer first.
  2754			 * This buffer will simulate a ring_buffer_event,
  2755			 * where the type_len is zero and the array[0] will
  2756			 * hold the full length.
  2757			 * (see include/linux/ring-buffer.h for details on
  2758			 *  how the ring_buffer_event is structured).
  2759			 *
  2760			 * Using a temp buffer during filtering and copying it
  2761			 * on a matched filter is quicker than writing directly
  2762			 * into the ring buffer and then discarding it when
  2763			 * it doesn't match. That is because the discard
  2764			 * requires several atomic operations to get right.
  2765			 * Copying on match and doing nothing on a failed match
  2766			 * is still quicker than no copy on match, but having
  2767			 * to discard out of the ring buffer on a failed match.
  2768			 */
> 2769			if (entry = __this_cpu_read(trace_buffered_event)) {
  2770				int max_len = PAGE_SIZE - struct_size(entry, array, 1);
  2771	
  2772				val = this_cpu_inc_return(trace_buffered_event_cnt);
  2773	
  2774				/*
  2775				 * Preemption is disabled, but interrupts and NMIs
  2776				 * can still come in now. If that happens after
  2777				 * the above increment, then it will have to go
  2778				 * back to the old method of allocating the event
  2779				 * on the ring buffer, and if the filter fails, it
  2780				 * will have to call ring_buffer_discard_commit()
  2781				 * to remove it.
  2782				 *
  2783				 * Need to also check the unlikely case that the
  2784				 * length is bigger than the temp buffer size.
  2785				 * If that happens, then the reserve is pretty much
  2786				 * guaranteed to fail, as the ring buffer currently
  2787				 * only allows events less than a page. But that may
  2788				 * change in the future, so let the ring buffer reserve
  2789				 * handle the failure in that case.
  2790				 */
  2791				if (val == 1 && likely(len <= max_len)) {
  2792					trace_event_setup(entry, type, trace_ctx);
  2793					entry->array[0] = len;
  2794					/* Return with preemption disabled */
  2795					return entry;
  2796				}
  2797				this_cpu_dec(trace_buffered_event_cnt);
  2798			}
  2799			/* __trace_buffer_lock_reserve() disables preemption */
  2800			preempt_enable_notrace();
  2801		}
  2802	
  2803		entry = __trace_buffer_lock_reserve(*current_rb, type, len,
  2804						    trace_ctx);
  2805		/*
  2806		 * If tracing is off, but we have triggers enabled
  2807		 * we still need to look at the event data. Use the temp_buffer
  2808		 * to store the trace event for the trigger to use. It's recursive
  2809		 * safe and will not be recorded anywhere.
  2810		 */
  2811		if (!entry && trace_file->flags & EVENT_FILE_FL_TRIGGER_COND) {
  2812			*current_rb = temp_buffer;
  2813			entry = __trace_buffer_lock_reserve(*current_rb, type, len,
  2814							    trace_ctx);
  2815		}
  2816		return entry;
  2817	}
  2818	EXPORT_SYMBOL_GPL(trace_event_buffer_lock_reserve);
  2819	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[PATCH 3/5] tracing: Have eprobes use filtering logic of trace events

[Steven Rostedt]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

The eprobes open code the reserving of the event on the ring buffer for
ftrace instead of using the ftrace event wrappers, which means that it
doesn't get affected by the filters, breaking the filtering logic on user
space.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_eprobe.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c
index 928867f527e7..88487752d307 100644
--- a/kernel/trace/trace_eprobe.c
+++ b/kernel/trace/trace_eprobe.c
@@ -489,18 +489,12 @@ __eprobe_trace_func(struct eprobe_data *edata, void *rec)
 	if (trace_trigger_soft_disabled(edata->file))
 		return;
 
-	fbuffer.trace_ctx = tracing_gen_ctx();
-	fbuffer.trace_file = edata->file;
-
 	dsize = get_eprobe_size(&edata->ep->tp, rec);
-	fbuffer.regs = NULL;
-
-	fbuffer.event =
-		trace_event_buffer_lock_reserve(&fbuffer.buffer, edata->file,
-					call->event.type,
-					sizeof(*entry) + edata->ep->tp.size + dsize,
-					fbuffer.trace_ctx);
-	if (!fbuffer.event)
+
+	entry = trace_event_buffer_reserve(&fbuffer, edata->file,
+					   sizeof(*entry) + edata->ep->tp.size + dsize);
+
+	if (!entry)
 		return;
 
 	entry = fbuffer.entry = ring_buffer_event_data(fbuffer.event);
-- 
2.33.0

[PATCH 4/5] tracing/kprobes: Do not open code event reserve logic

[Steven Rostedt]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

As kprobe events use trace_event_buffer_commit() to commit the event to
the ftrace ring buffer, for consistency, it should use
trace_event_buffer_reserve() to allocate it, as the two functions are
related.

Cc: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_kprobe.c | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 33272a7b6912..d10c01948e68 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1383,17 +1383,11 @@ __kprobe_trace_func(struct trace_kprobe *tk, struct pt_regs *regs,
 	if (trace_trigger_soft_disabled(trace_file))
 		return;
 
-	fbuffer.trace_ctx = tracing_gen_ctx();
-	fbuffer.trace_file = trace_file;
-
 	dsize = __get_data_size(&tk->tp, regs);
 
-	fbuffer.event =
-		trace_event_buffer_lock_reserve(&fbuffer.buffer, trace_file,
-					call->event.type,
-					sizeof(*entry) + tk->tp.size + dsize,
-					fbuffer.trace_ctx);
-	if (!fbuffer.event)
+	entry = trace_event_buffer_reserve(&fbuffer, trace_file,
+					   sizeof(*entry) + tk->tp.size + dsize);
+	if (!entry)
 		return;
 
 	fbuffer.regs = regs;
@@ -1430,16 +1424,11 @@ __kretprobe_trace_func(struct trace_kprobe *tk, struct kretprobe_instance *ri,
 	if (trace_trigger_soft_disabled(trace_file))
 		return;
 
-	fbuffer.trace_ctx = tracing_gen_ctx();
-	fbuffer.trace_file = trace_file;
-
 	dsize = __get_data_size(&tk->tp, regs);
-	fbuffer.event =
-		trace_event_buffer_lock_reserve(&fbuffer.buffer, trace_file,
-					call->event.type,
-					sizeof(*entry) + tk->tp.size + dsize,
-					fbuffer.trace_ctx);
-	if (!fbuffer.event)
+
+	entry = trace_event_buffer_reserve(&fbuffer, trace_file,
+					   sizeof(*entry) + tk->tp.size + dsize);
+	if (!entry)
 		return;
 
 	fbuffer.regs = regs;
-- 
2.33.0

Re: [PATCH 4/5] tracing/kprobes: Do not open code event reserve logic

[Masami Hiramatsu]

On Mon, 29 Nov 2021 21:39:49 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> 
> As kprobe events use trace_event_buffer_commit() to commit the event to
> the ftrace ring buffer, for consistency, it should use
> trace_event_buffer_reserve() to allocate it, as the two functions are
> related.

OK, I got the rule.

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

BTW, what should be used for trace_event_buffer_lock_reserve() as a pair call?

Thanks!

> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
>  kernel/trace/trace_kprobe.c | 25 +++++++------------------
>  1 file changed, 7 insertions(+), 18 deletions(-)
> 
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 33272a7b6912..d10c01948e68 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -1383,17 +1383,11 @@ __kprobe_trace_func(struct trace_kprobe *tk, struct pt_regs *regs,
>  	if (trace_trigger_soft_disabled(trace_file))
>  		return;
>  
> -	fbuffer.trace_ctx = tracing_gen_ctx();
> -	fbuffer.trace_file = trace_file;
> -
>  	dsize = __get_data_size(&tk->tp, regs);
>  
> -	fbuffer.event =
> -		trace_event_buffer_lock_reserve(&fbuffer.buffer, trace_file,
> -					call->event.type,
> -					sizeof(*entry) + tk->tp.size + dsize,
> -					fbuffer.trace_ctx);
> -	if (!fbuffer.event)
> +	entry = trace_event_buffer_reserve(&fbuffer, trace_file,
> +					   sizeof(*entry) + tk->tp.size + dsize);
> +	if (!entry)
>  		return;
>  
>  	fbuffer.regs = regs;
> @@ -1430,16 +1424,11 @@ __kretprobe_trace_func(struct trace_kprobe *tk, struct kretprobe_instance *ri,
>  	if (trace_trigger_soft_disabled(trace_file))
>  		return;
>  
> -	fbuffer.trace_ctx = tracing_gen_ctx();
> -	fbuffer.trace_file = trace_file;
> -
>  	dsize = __get_data_size(&tk->tp, regs);
> -	fbuffer.event =
> -		trace_event_buffer_lock_reserve(&fbuffer.buffer, trace_file,
> -					call->event.type,
> -					sizeof(*entry) + tk->tp.size + dsize,
> -					fbuffer.trace_ctx);
> -	if (!fbuffer.event)
> +
> +	entry = trace_event_buffer_reserve(&fbuffer, trace_file,
> +					   sizeof(*entry) + tk->tp.size + dsize);
> +	if (!entry)
>  		return;
>  
>  	fbuffer.regs = regs;
> -- 
> 2.33.0


-- 
Masami Hiramatsu <mhiramat@kernel.org>

[PATCH 5/5] tracing/uprobes: Use trace_event_buffer_reserve() helper

[Steven Rostedt]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

To be consistent with kprobes and eprobes, use
trace_event_buffer_reserver() and trace_event_buffer_commit(). This will
ensure that any updates to trace events will also be implemented on uprobe
events.

Cc: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_uprobe.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index f5f0039d31e5..177717c4e74a 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -949,8 +949,8 @@ static void __uprobe_trace_func(struct trace_uprobe *tu,
 				struct trace_event_file *trace_file)
 {
 	struct uprobe_trace_entry_head *entry;
+	struct trace_event_buffer fbuffer;
 	struct trace_buffer *buffer;
-	struct ring_buffer_event *event;
 	void *data;
 	int size, esize;
 	struct trace_event_call *call = trace_probe_event_call(&tu->tp);
@@ -965,12 +965,10 @@ static void __uprobe_trace_func(struct trace_uprobe *tu,
 
 	esize = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
 	size = esize + tu->tp.size + dsize;
-	event = trace_event_buffer_lock_reserve(&buffer, trace_file,
-						call->event.type, size, 0);
-	if (!event)
+	entry = trace_event_buffer_reserve(&fbuffer, trace_file, size);
+	if (!entry)
 		return;
 
-	entry = ring_buffer_event_data(event);
 	if (is_ret_probe(tu)) {
 		entry->vaddr[0] = func;
 		entry->vaddr[1] = instruction_pointer(regs);
@@ -982,7 +980,7 @@ static void __uprobe_trace_func(struct trace_uprobe *tu,
 
 	memcpy(data, ucb->buf, tu->tp.size + dsize);
 
-	event_trigger_unlock_commit(trace_file, buffer, event, entry, 0);
+	trace_event_buffer_commit(&fbuffer);
 }
 
 /* uprobe handler */
-- 
2.33.0

Re: [PATCH 5/5] tracing/uprobes: Use trace_event_buffer_reserve() helper

[Masami Hiramatsu]

On Mon, 29 Nov 2021 21:39:50 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> 
> To be consistent with kprobes and eprobes, use
> trace_event_buffer_reserver() and trace_event_buffer_commit(). This will
> ensure that any updates to trace events will also be implemented on uprobe
> events.
> 

Ah, event_trigger_unlock_commit() is the pair commit function. I got it.
Anyway, the pair of trace_event_buffer_reserve() and trace_event_buffer_commit()
should be used.

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

Thank you,

> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
>  kernel/trace/trace_uprobe.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
> index f5f0039d31e5..177717c4e74a 100644
> --- a/kernel/trace/trace_uprobe.c
> +++ b/kernel/trace/trace_uprobe.c
> @@ -949,8 +949,8 @@ static void __uprobe_trace_func(struct trace_uprobe *tu,
>  				struct trace_event_file *trace_file)
>  {
>  	struct uprobe_trace_entry_head *entry;
> +	struct trace_event_buffer fbuffer;
>  	struct trace_buffer *buffer;
> -	struct ring_buffer_event *event;
>  	void *data;
>  	int size, esize;
>  	struct trace_event_call *call = trace_probe_event_call(&tu->tp);
> @@ -965,12 +965,10 @@ static void __uprobe_trace_func(struct trace_uprobe *tu,
>  
>  	esize = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
>  	size = esize + tu->tp.size + dsize;
> -	event = trace_event_buffer_lock_reserve(&buffer, trace_file,
> -						call->event.type, size, 0);
> -	if (!event)
> +	entry = trace_event_buffer_reserve(&fbuffer, trace_file, size);
> +	if (!entry)
>  		return;
>  
> -	entry = ring_buffer_event_data(event);
>  	if (is_ret_probe(tu)) {
>  		entry->vaddr[0] = func;
>  		entry->vaddr[1] = instruction_pointer(regs);
> @@ -982,7 +980,7 @@ static void __uprobe_trace_func(struct trace_uprobe *tu,
>  
>  	memcpy(data, ucb->buf, tu->tp.size + dsize);
>  
> -	event_trigger_unlock_commit(trace_file, buffer, event, entry, 0);
> +	trace_event_buffer_commit(&fbuffer);
>  }
>  
>  /* uprobe handler */
> -- 
> 2.33.0


-- 
Masami Hiramatsu <mhiramat@kernel.org>