[PATCH 0/2] Introduce new API to set synthetic event instance

[PATCH 0/2] Introduce new API to set synthetic event instance

[Tzvetomir Stoyanov (VMware)]

A new API tracefs_synth_set_instance() to set the trace instance, where the
synthetic event is created - the trigger files of the start and end events.

Tzvetomir Stoyanov (VMware) (2):
  libtracefs: New API to set synthetic event instance
  libtracefs: Documentation for tracefs_synth_set_instance

 Documentation/libtracefs-synth2.txt | 15 +++++++++++----
 Documentation/libtracefs.txt        |  1 +
 include/tracefs.h                   |  1 +
 src/tracefs-hist.c                  | 19 +++++++++++++++++++
 4 files changed, 32 insertions(+), 4 deletions(-)

-- 
2.39.2

[PATCH 1/2] libtracefs: New API to set synthetic event instance

[Tzvetomir Stoyanov (VMware)]

The current implementation of tracefs_synth_create() API always creates
the synthetic event in the top trace instance - the "trigger" files of
the start and end events are updated in the context of the top instance.
Then the same synthetic event can be enabled and operates in any trace
instance. That logic works well in the most use cases, where there is a
single application, managing these events and the trace process.
However, there are use cases where multiple applications can run
different trace sessions, each in its own trace instance. For those use
cases, the default logic does not work well - the synthetic event can be
created by one application in the top instance, and cannot be reused
easily by any other applications in different trace instance.
The tracefs_synth structure already has an instance context, but there
is no way to set it to other than default top instance. That's why a new
API is introduced: tracefs_synth_set_instance(), which sets the instance
in the tracefs_synth structure. If that API is called before
tracefs_synth_create(), then the "trigger" files will be updated only in
the context of that user instance, instead of the default one. This
allows synthetic events to be more flexible and reduces the interference
between different trace instance.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/tracefs.h  |  1 +
 src/tracefs-hist.c | 19 +++++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/include/tracefs.h b/include/tracefs.h
index 3547b5a..9a53e0e 100644
--- a/include/tracefs.h
+++ b/include/tracefs.h
@@ -607,6 +607,7 @@ struct tracefs_hist *tracefs_synth_get_start_hist(struct tracefs_synth *synth);
 int tracefs_synth_create(struct tracefs_synth *synth);
 int tracefs_synth_destroy(struct tracefs_synth *synth);
 void tracefs_synth_free(struct tracefs_synth *synth);
+int tracefs_synth_set_instance(struct tracefs_synth *synth, struct tracefs_instance *instance);
 int tracefs_synth_echo_cmd(struct trace_seq *seq, struct tracefs_synth *synth);
 int tracefs_synth_raw_fmt(struct trace_seq *seq, struct tracefs_synth *synth);
 const char *tracefs_synth_show_event(struct tracefs_synth *synth);
diff --git a/src/tracefs-hist.c b/src/tracefs-hist.c
index fb6231e..9141bbb 100644
--- a/src/tracefs-hist.c
+++ b/src/tracefs-hist.c
@@ -2178,6 +2178,25 @@ tracefs_synth_get_start_hist(struct tracefs_synth *synth)
 	return hist;
 }
 
+/**
+ * tracefs_synth_set_instance - Set the ftrace instance of the synthetic events
+ * @synth: The tracefs_synth descriptor
+ * @instance: ftrace instance
+ *
+ * Set the ftrace instance, in which the synthetic event will be created. By default,
+ * the top instance is used. This API must be called before the call to tracefs_synth_create(),
+ * in order to use the new instance when creating the event.
+ *
+ * Returns 0 on success and -1 on error.
+ */
+int tracefs_synth_set_instance(struct tracefs_synth *synth, struct tracefs_instance *instance)
+{
+	if (!synth)
+		return -1;
+	synth->instance = instance;
+	return 0;
+}
+
 /**
  * tracefs_synth_create - creates the synthetic event on the system
  * @synth: The tracefs_synth descriptor
-- 
2.39.2

[PATCH 2/2] libtracefs: Documentation for tracefs_synth_set_instance

[Tzvetomir Stoyanov (VMware)]

The newly introduced API tracefs_synth_set_instance() should be
described in the man pages of the tracefs library.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 Documentation/libtracefs-synth2.txt | 15 +++++++++++----
 Documentation/libtracefs.txt        |  1 +
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/Documentation/libtracefs-synth2.txt b/Documentation/libtracefs-synth2.txt
index 7e8e6cc..d94fa7c 100644
--- a/Documentation/libtracefs-synth2.txt
+++ b/Documentation/libtracefs-synth2.txt
@@ -4,7 +4,7 @@ libtracefs(3)
 NAME
 ----
 tracefs_synth_create, tracefs_synth_destroy, tracefs_synth_complete,
-tracefs_synth_trace, tracefs_synth_snapshot, tracefs_synth_save
+tracefs_synth_trace, tracefs_synth_snapshot, tracefs_synth_save,tracefs_synth_set_instance,
 - Creation of synthetic events
 
 SYNOPSIS
@@ -17,6 +17,7 @@ int *tracefs_synth_create*(struct tracefs_synth pass:[*]_synth_);
 int *tracefs_synth_destroy*(struct tracefs_synth pass:[*]_synth_);
 bool *tracefs_synth_complete*(struct tracefs_synth pass:[*]_synth_);
 
+int *tracefs_synth_set_instance*(struct tracefs_synth pass:[*]_synth_, struct tracefs_instance pass:[*]_instance_);
 int *tracefs_synth_trace*(struct tracefs_synth pass:[*]_synth_,
 			enum tracefs_synth_handler _type_, const char pass:[*]_var_);
 int *tracefs_synth_snapshot*(struct tracefs_synth pass:[*]_synth_,
@@ -45,9 +46,11 @@ as a field for both events to calculate the delta in nanoseconds, or use
 *TRACEFS_TIMESTAMP_USECS* as the compare fields for both events to calculate the
 delta in microseconds. This is used as the example below.
 
-*tracefs_synth_create*() creates the synthetic event in the system. The synthetic events apply
-across all instances. A synthetic event must be created with *tracefs_synth_alloc*(3) before
-it can be created.
+*tracefs_synth_create*() creates the synthetic event in the system. By default, the synthetic
+events are created in the top trace instance and apply across all instances.
+The *tracefs_synth_set_instance()* API can be used to set a custom instance, where the synthetic
+event will be created. In that case the event operates only in that instance. A synthetic event
+must be created with *tracefs_synth_alloc*(3) before it can be created.
 
 *tracefs_synth_destroy*() destroys the synthetic event. It will attempt to stop the running of it in
 its instance (top by default), but if its running in another instance this may fail as busy.
@@ -74,6 +77,10 @@ then save the given _save_fields_ list. The fields will be stored in the histogr
 "hist" file of the event that can be retrieved with *tracefs_event_file_read*(3).
 _var_ must be one of the _name_ elements used in *tracefs_synth_add_end_field*(3).
 
+*tracefs_synth_set_instance()* Set the trace instance, where the synthetic event will be
+created. By default, the top instance is used. This API must be called before the call to
+*tracefs_synth_create()*, in order to use the new instance when creating the event.
+
 RETURN VALUE
 ------------
 All functions return zero on success or -1 on error.
diff --git a/Documentation/libtracefs.txt b/Documentation/libtracefs.txt
index c3f448d..daf044f 100644
--- a/Documentation/libtracefs.txt
+++ b/Documentation/libtracefs.txt
@@ -213,6 +213,7 @@ Synthetic events:
 	void *tracefs_synth_free*(struct tracefs_synth pass:[*]_synth_);
 	int *tracefs_synth_create*(struct tracefs_synth pass:[*]_synth_);
 	int *tracefs_synth_destroy*(struct tracefs_synth pass:[*]_synth_);
+	int *tracefs_synth_set_instance*(struct tracefs_synth pass:[*]_synth_, struct tracefs_instance pass:[*]_instance_);
 	int *tracefs_synth_echo_cmd*(struct trace_seq pass:[*]_seq_, struct tracefs_synth pass:[*]_synth_);
 	bool *tracefs_synth_complete*(struct tracefs_synth pass:[*]_synth_);
 	struct tracefs_hist pass:[*]*tracefs_synth_get_start_hist*(struct tracefs_synth pass:[*]_synth_);
-- 
2.39.2

Re: [PATCH 2/2] libtracefs: Documentation for tracefs_synth_set_instance

[Steven Rostedt]

On Tue, 28 Mar 2023 18:03:21 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> @@ -45,9 +46,11 @@ as a field for both events to calculate the delta in nanoseconds, or use
>  *TRACEFS_TIMESTAMP_USECS* as the compare fields for both events to calculate the
>  delta in microseconds. This is used as the example below.
>  
> -*tracefs_synth_create*() creates the synthetic event in the system. The synthetic events apply
> -across all instances. A synthetic event must be created with *tracefs_synth_alloc*(3) before
> -it can be created.
> +*tracefs_synth_create*() creates the synthetic event in the system. By default, the synthetic
> +events are created in the top trace instance and apply across all instances.
> +The *tracefs_synth_set_instance()* API can be used to set a custom instance, where the synthetic
> +event will be created. In that case the event operates only in that instance. A synthetic event

The above is somewhat incorrect. That makes it sound like you can not
use the event elsewhere, where you most certainly can.

 ># cd /sys/kernel/tracing
 ># mkdir instances/foo
 ># echo 'wakeup pid_t pid; u64 lat' > synthetic_events
 ># echo 'hist:keys=pid:ts=common_timestamp.usecs' > instances/foo/events/sched/sched_waking/trigger
 ># echo 'hist:keys=next_pid:lat=common_timestamp.usecs-$ts:onmatch(sched.sched_waking).trace(wakeup,next_pid,$lat)' > instances/foo/events/sched/sched_switch/trigger
 ># mkdir instances/bar
 ># echo 1 > instances/bar/events/synthetic/wakeup/enable
 ># cat instances/bar/trace
# tracer: nop
#
# entries-in-buffer/entries-written: 73761/73761   #P:8
#
#                                _-----=> irqs-off/BH-disabled
#                               / _----=> need-resched
#                              | / _---=> hardirq/softirq
#                              || / _--=> preempt-depth
#                              ||| / _-=> migrate-disable
#                              |||| /     delay
#           TASK-PID     CPU#  |||||  TIMESTAMP  FUNCTION
#              | |         |   |||||     |         |
          <idle>-0       [001] d..4. 56208.972686: wakeup: pid=1352 lat=25
            bash-843     [002] d..4. 56208.972784: wakeup: pid=1353 lat=20
          <idle>-0       [004] d..4. 56208.972894: wakeup: pid=3408 lat=25
          <idle>-0       [006] d..4. 56208.972937: wakeup: pid=820 lat=33
          <idle>-0       [004] d..4. 56208.972951: wakeup: pid=3408 lat=23
          <idle>-0       [004] d..4. 56208.972993: wakeup: pid=1355 lat=12
          <idle>-0       [003] d..4. 56208.973030: wakeup: pid=1354 lat=26


Now, I am going to apply these patches, but I also will update them to
reflect the actual benefit of them. The issue I see is that you have
triggers in the toplevel (histograms). And perhaps you don't want those
histograms there, and want to keep the histograms used to create a
synthetic event in a particular instance. That makes more sense,
although it's not that much of a interference, as you can add more than
one histogram to an event.

 ># cat instances/foo/events/sched/sched_waking/hist
# event histogram
#
# trigger info: hist:keys=pid:vals=hitcount:ts=common_timestamp.usecs:sort=hitcount:size=2048:clock=global [active]
#

{ pid:          2 } hitcount:          1
{ pid:         41 } hitcount:          1
{ pid:       3609 } hitcount:          1
{ pid:         26 } hitcount:          1
{ pid:       3612 } hitcount:          1
{ pid:         15 } hitcount:          1
[..]
{ pid:        289 } hitcount:       1920
{ pid:        454 } hitcount:       3747
{ pid:         84 } hitcount:       4714
{ pid:       3257 } hitcount:      83897
{ pid:       1351 } hitcount:     681823
{ pid:       1357 } hitcount:     681830
{ pid:       1353 } hitcount:     681831
{ pid:       1352 } hitcount:     681834
{ pid:       1358 } hitcount:     681837
{ pid:       1356 } hitcount:     681869
{ pid:       1355 } hitcount:     681872
{ pid:       1354 } hitcount:     681878

Totals:
    Hits: 5564963
    Entries: 89
    Dropped: 0

 ># cat events/sched/sched_waking/hist
 >#

-- Steve


> +must be created with *tracefs_synth_alloc*(3) before it can be created.
>