On Thu, Nov 4, 2021 at 7:13 PM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
>
[ ... ]
> > static int remove_hist(struct tracefs_instance *instance,
> > struct tep_event *event, const char *hist)
> > {
> > @@ -1919,7 +1911,6 @@ tracefs_synth_get_start_hist(struct tracefs_synth *synth)
> > int tracefs_synth_create(struct tracefs_instance *instance,
> > struct tracefs_synth *synth)
>
> I think we should call this:
>
> tracefs_synth_create_raw()
>
> that passes in the "instance".
>
> Then we could have tracefs_synth_create() just use the top instance (no
> instance passed to it), and we could decide later if we want to create an
> internal instance or not to do the work to keep from modifying the top
> instance.
>
> That is, we would have:
>
> int tracefs_synth_create(struct tracefs_synth *synth)
> {
> return tracefs_synth_create_raw(synth->create_instance, synth);
> }
>
> Have synth->create_instance be defaulted to NULL on allocation, and would
> get updated by another interface perhaps?
>
> We could also add to the _raw() function:
>
> if (synth->create_instance && instance &&
> synth->create_instance != instance) {
> errno = EINVAL;
> return -1
> }
>
> if (!synth->create_instance && instance) {
> syth->create_instance = instance;
> trace_get_instance(instance);
> }
>
> Would need to have in the destructor:
>
> if (synth->create_instance)
> trace_put_instance(synth->create_instance);
>
>
> Or something like that.
>
I looked and tested more this code today, but as I understood the
instance is mandatory in histograms creation and should not be removed
from this API. That API appends the histogram config in the event's
trigger files, which is for specific instances. The histogram is
accumulated only in the context of that instance. I'm going to send
next version of the patch set, but without changes to this API.
>
[ ... ]
>
--
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center
|