On Fri, 12 Nov 2021 07:04:36 +0200
Tzvetomir Stoyanov <tz.stoyanov@xxxxxxxxx> wrote:
> 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:
No, it does not. This is an internal issue, let's not burden the users with
extra information they need to manage.
The problem goes away with the following patch:
diff --git a/src/tracefs-dynevents.c b/src/tracefs-dynevents.c
index f525e8d..f2a6c8d 100644
--- a/src/tracefs-dynevents.c
+++ b/src/tracefs-dynevents.c
@@ -41,16 +41,14 @@ struct dyn_events_desc {
int (*parse)(struct dyn_events_desc *desc, const char *group,
char *line, struct tracefs_dynevent **ret_dyn);
} dynevents[] = {
- {TRACEFS_DYNEVENT_KPROBE, NULL, "p", dyn_generic_del, dyn_generic_parse},
- {TRACEFS_DYNEVENT_KRETPROBE, NULL, "r", dyn_generic_del, dyn_generic_parse},
- {TRACEFS_DYNEVENT_UPROBE, NULL, "p", dyn_generic_del, dyn_generic_parse},
- {TRACEFS_DYNEVENT_URETPROBE, NULL, "r", dyn_generic_del, dyn_generic_parse},
- {TRACEFS_DYNEVENT_EPROBE, NULL, "e", dyn_generic_del, dyn_generic_parse},
- {TRACEFS_DYNEVENT_SYNTH, NULL, "s", dyn_synth_del, dyn_synth_parse},
+ {TRACEFS_DYNEVENT_KPROBE, KPROBE_EVENTS, "p", dyn_generic_del, dyn_generic_parse},
+ {TRACEFS_DYNEVENT_KRETPROBE, KPROBE_EVENTS, "r", dyn_generic_del, dyn_generic_parse},
+ {TRACEFS_DYNEVENT_UPROBE, UPROBE_EVENTS, "p", dyn_generic_del, dyn_generic_parse},
+ {TRACEFS_DYNEVENT_URETPROBE, UPROBE_EVENTS, "r", dyn_generic_del, dyn_generic_parse},
+ {TRACEFS_DYNEVENT_EPROBE, SYNTH_EVENTS, "e", dyn_generic_del, dyn_generic_parse},
+ {TRACEFS_DYNEVENT_SYNTH, "", "s", dyn_synth_del, dyn_synth_parse},
};
-
-
static int dyn_generic_del(struct dyn_events_desc *desc, struct tracefs_dynevent *dyn)
{
char *str;
@@ -282,19 +280,6 @@ static void init_devent_desc(void)
dynevents[i].file = DYNEVENTS_EVENTS;
return;
}
-
- if (tracefs_file_exists(NULL, KPROBE_EVENTS)) {
- 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)) {
- 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)) {
- 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)
Let the if the files do not exist, then let the failure happen when the the
user tries to create the event in the system. The external directory is
only for reading, not for creating.
-- Steve
|