From: Tzvetomir Stoyanov <tz.stoyanov@gmail.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Yordan Karadzhov <y.karadz@gmail.com>,
	Linux Trace Devel <linux-trace-devel@vger.kernel.org>
Subject: Re: [PATCH v4 08/10] libtracefs: Use the internal dynamic events API when creating synthetic events
Date: Fri, 5 Nov 2021 14:15:05 +0200	[thread overview]
Message-ID: <CAPpZLN5bSdCKOvAMSefAaj0osq60qgcyRE7npMS6jPOcYoEAYg@mail.gmail.com> (raw)
In-Reply-To: <20211104131351.1110c122@gandalf.local.home>
On Thu, Nov 4, 2021 at 7:13 PM Steven Rostedt <rostedt@goodmis.org> 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
next prev parent reply	other threads:[~2021-11-05 12:15 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-04 11:10 [PATCH v4 00/10] libtracefs dynamic events support Tzvetomir Stoyanov (VMware)
2021-11-04 11:10 ` [PATCH v4 01/10] libtracefs: New APIs for dynamic events Tzvetomir Stoyanov (VMware)
2021-11-04 13:58   ` Steven Rostedt
2021-11-04 14:37   ` Steven Rostedt
2021-11-04 14:54   ` Steven Rostedt
2021-11-04 11:10 ` [PATCH v4 02/10] libtracefs: New APIs for kprobe allocation Tzvetomir Stoyanov (VMware)
2021-11-04 16:29   ` Steven Rostedt
2021-11-04 11:10 ` [PATCH v4 03/10] libtracefs: Remove redundant kprobes APIs Tzvetomir Stoyanov (VMware)
2021-11-04 11:10 ` [PATCH v4 04/10] libtracefs: Change tracefs_kprobe_info API Tzvetomir Stoyanov (VMware)
2021-11-04 16:33   ` Steven Rostedt
2021-11-04 17:28     ` Tzvetomir Stoyanov
2021-11-04 17:57       ` Steven Rostedt
2021-11-04 11:10 ` [PATCH v4 05/10] libtracefs: Reimplement kprobe raw APIs Tzvetomir Stoyanov (VMware)
2021-11-04 11:10 ` [PATCH v4 06/10] libtracefs: Extend kprobes unit test Tzvetomir Stoyanov (VMware)
2021-11-04 11:10 ` [PATCH v4 07/10] libtracefs: Rename tracefs_synth_init API Tzvetomir Stoyanov (VMware)
2021-11-04 11:10 ` [PATCH v4 08/10] libtracefs: Use the internal dynamic events API when creating synthetic events Tzvetomir Stoyanov (VMware)
2021-11-04 12:47   ` Yordan Karadzhov
2021-11-04 13:21     ` Steven Rostedt
2021-11-04 17:13   ` Steven Rostedt
2021-11-05 12:15     ` Tzvetomir Stoyanov [this message]
2021-11-05 12:33       ` Steven Rostedt
2021-11-04 11:10 ` [PATCH v4 09/10] libtracefs: Update kprobes man pages Tzvetomir Stoyanov (VMware)
2021-11-04 20:28   ` Steven Rostedt
2021-11-04 11:10 ` [PATCH v4 10/10] libtracefs: Document dynamic events APIs Tzvetomir Stoyanov (VMware)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox
  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):
  git send-email \
    --in-reply-to=CAPpZLN5bSdCKOvAMSefAaj0osq60qgcyRE7npMS6jPOcYoEAYg@mail.gmail.com \
    --to=tz.stoyanov@gmail.com \
    --cc=linux-trace-devel@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=y.karadz@gmail.com \
    /path/to/YOUR_REPLY
  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
  Be sure your reply has a Subject: header at the top and a blank line
  before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).