Hi Steven,
On Fri, Nov 12, 2021 at 12:33 AM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
>
> After applying these patches, I tested sqlhist and it's broken.
>
> What I do is:
>
> (as root)
> # mkdir /tmp/tracing
> # cp -a /sys/kernel/tracing/events /tmp/tracing
>
> (as my user)
> $ ./sqlhist -t /tmp/tracing/ -n waking 'select start.pid,
> (end.TIMESTAMP_USECS - start.TIMESTAMP_USECS) as delta from sched_waking as start join sched_switch as end on start.pid = end.next_pid'
>
> Which prints nothing, but before applying the patches, I would have this:
>
> echo 'waking s32 pid; u64 delta;' > /sys/kernel/tracing/synthetic_events
> echo 'hist:keys=pid:__arg_15772_1=pid,__arg_15772_2=common_timestamp.usecs' > /sys/kernel/tracing/events/sched/sched_waking/trigger
> echo 'hist:keys=next_pid:pid=$__arg_15772_1,delta=common_timestamp.usecs-$__arg_15772_2:onmatch(sched.sched_waking).trace(waking,$pid,$delta)' > /sys/kernel/tracing/events/sched/sched_switch/trigger
>
> Debugging it, I found that it's due to this change.
>
I totally missed the custom trace dir use case. The dynevents APIs
should be extended with additional parameter, to handle that use case.
I see two ways to do this:
1. Passing a tracefs_instance* parameter to those APIs. This can be
NULL for top instance in default tracing directory, or allocated with:
tracefs_instance_alloc("/tmp/tracing/", NULL) - for the instance
in a custom tracing directory.
This approach adds flexibility to work with any non default
instance. It also resolves that question with the instance needed by
the histograms configuration.
2. Passing a string parameter with custom trace directory path, or
NULL for default system trace directory.
Those APIs should be extended with that additional parameter:
tracefs_dynevent_get_all()
tracefs_dynevent_destroy_all()
tracefs_kprobe_alloc()
tracefs_kretprobe_alloc()
tracefs_kprobe_raw()
tracefs_kretprobe_raw()
tracefs_synth_alloc()
tracefs_sql()
>
> On Mon, 8 Nov 2021 10:03:53 +0200
> "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@xxxxxxxxx> wrote:
>
> > +static void init_devent_desc(void)
> > +{
> > + int i;
> > +
> > + BUILD_BUG_ON(ARRAY_SIZE(dynevents) != EVENT_INDEX(TRACEFS_DYNEVENT_MAX));
> > +
> > + /* Use ftrace dynamic_events, if available */
> > + if (tracefs_file_exists(NULL, DYNEVENTS_EVENTS)) {
>
> // fails due to permission denied.
>
> > + for (i = 0; i < EVENT_INDEX(TRACEFS_DYNEVENT_MAX); i++)
> > + dynevents[i].file = DYNEVENTS_EVENTS;
> > + return;
> > + }
> > +
> > + if (tracefs_file_exists(NULL, KPROBE_EVENTS)) {
>
> // fails due to permission denied.
>
> > + dynevents[EVENT_INDEX(TRACEFS_DYNEVENT_KPROBE)].file = KPROBE_EVENTS;
> > + dynevents[EVENT_INDEX(TRACEFS_DYNEVENT_KRETPROBE)].file = KPROBE_EVENTS;
> > + }
> > + if (tracefs_file_exists(NULL, UPROBE_EVENTS)) {
>
> // fails due to permission denied.
>
> > + dynevents[EVENT_INDEX(TRACEFS_DYNEVENT_UPROBE)].file = UPROBE_EVENTS;
> > + dynevents[EVENT_INDEX(TRACEFS_DYNEVENT_URETPROBE)].file = UPROBE_EVENTS;
> > + }
> > + if (tracefs_file_exists(NULL, SYNTH_EVENTS)) {
>
> // fails due to permission denied.
>
> > + dynevents[EVENT_INDEX(TRACEFS_DYNEVENT_SYNTH)].file = SYNTH_EVENTS;
> > + dynevents[EVENT_INDEX(TRACEFS_DYNEVENT_SYNTH)].prefix = "";
> > + }
> > +}
> > +
> > +static struct dyn_events_desc *get_devent_desc(enum tracefs_dynevent_type type)
> > +{
> > +
> > + static bool init;
> > +
> > + if (type >= TRACEFS_DYNEVENT_MAX)
> > + return NULL;
> > +
> > + if (!init) {
> > + init_devent_desc();
> > + init = true;
> > + }
> > +
> > + return &dynevents[EVENT_INDEX(type)];
> > +}
> > +
> > +/**
> > + * dynevent_alloc - Allocate new dynamic event
> > + * @type: Type of the dynamic event
> > + * @system: The system name (NULL for the default dynamic)
> > + * @event: Name of the event
> > + * @addr: The function and offset (or address) to insert the probe
> > + * @format: The format string to define the probe.
> > + *
> > + * Allocate a dynamic event context that will be in the @system group
> > + * (or dynamic if @system is NULL). Have the name of @event and
> > + * will be associated to @addr, if applicable for that event type
> > + * (function name, with or without offset, or a address). And the @format will
> > + * define the format of the kprobe.
> > + * The dynamic event is not created in the system.
> > + *
> > + * Return a pointer to a dynamic event context on success, or NULL on error.
> > + * The returned pointer must be freed with tracefs_dynevent_free()
> > + *
> > + * errno will be set to EINVAL if event is NULL.
> > + */
> > +__hidden struct tracefs_dynevent *
> > +dynevent_alloc(enum tracefs_dynevent_type type, const char *system,
> > + const char *event, const char *address, const char *format)
> > +{
> > + struct tracefs_dynevent *devent;
> > + struct dyn_events_desc *desc;
> > +
> > + if (!event) {
> > + errno = EINVAL;
> > + return NULL;
> > + }
> > +
> > + desc = get_devent_desc(type);
>
> desc->file is NULL.
>
> > + if (!desc || !desc->file) {
> > + errno = ENOTSUP;
>
> Returns an error, when it use to work.
>
> > + return NULL;
> > + }
>
> -- Steve
--
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center
|