From: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
To: Ingo Molnar <mingo@kernel.org>, masami.hiramatsu.pt@hitachi.com
Cc: mingo@redhat.com, acme@kernel.org, brendan.d.gregg@gmail.com,
peterz@infradead.org, alexander.shishkin@linux.intel.com,
wangnan0@huawei.com, jolsa@kernel.org, ak@linux.intel.com,
treeze.taeung@gmail.com, mathieu.poirier@linaro.org,
hekuang@huawei.com, sukadev@linux.vnet.ibm.com,
ananth@in.ibm.com, naveen.n.rao@linux.vnet.ibm.com,
colin.ing@canonical.com, adrian.hunter@intel.com,
linux-kernel@vger.kernel.org, hemant@linux.vnet.ibm.com,
Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
Subject: Re: [RFC] perf/sdt: Directly record SDT event with 'perf record'
Date: Mon, 20 Feb 2017 16:31:50 +0530 [thread overview]
Message-ID: <58AACC9E.8070009@linux.vnet.ibm.com> (raw)
In-Reply-To: <20170220084224.GA24404@gmail.com>
Thanks Ingo,
On Monday 20 February 2017 02:12 PM, Ingo Molnar wrote:
> * Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote:
>
>> What should be the behavior of the tool? Should it record only one
>> 'sdt_libpthread:mutex_entry' which exists in uprobe_events? Or it
>> should record all the SDT events from libpthread? We can choose either
>> of two but both the cases are ambiguous.
> They are not ambiguous really if coded right: just pick one of the outcomes and
> maybe print a warning to inform the user that something weird is going on because
> not all markers are enabled?
>
> As a user I'd expect 'perf record' to enable all markers and print a warning that
> the markers were in a partial state. This would result in consistent behaviour.
Yes, makes sense.
> Does it make sense to only enable some of the markers that alias on the same name?
> If not then maybe disallow that in perf probe - or change perf probe to do the
> same thing as perf record.
'perf probe' is doing that correctly. It fetches all events with given name from
probe-cache and creates entries for them in uprobe_events.
The problem is the 2-step process of adding probes and then recording,
allowing users to select individual markers to record on.
>
> I.e. this is IMHO an artificial problem that users should not be exposed to and
> which can be solved by tooling.
>
> In particular if it's possible to enable only a part of the markers then perf
> record not continuing would be a failure mode: if for example a previous perf
> record session segfaulted (or ran out of RAM or was killed in the wrong moment or
> whatever) then it would not be possible to (easily) clean up the mess.
Agreed. We need to make this more robust.
>
>> Not allowing 'perf probe' for SDT event will solve all such issues.
>> Also it will make user interface simple and consistent. Other current
>> tooling (systemtap, for instance) also do not allow probing individual
>> markers when there are multiple markers with the same name.
> In any case if others agree with your change in UI flow too then it's fine by me,
> but please make it robust, i.e. if perf record sees partially enabled probes it
> should still continue.
@Masami, can you please provide your thoughts as well.
Thanks,
Ravi
next prev parent reply other threads:[~2017-02-20 13:19 UTC|newest]
Thread overview: 77+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-16 23:55 [PATCH 0/2] perf: add support of SDT probes arguments Alexis Berlemont
2016-11-16 23:56 ` [PATCH 1/2] perf sdt: add scanning of sdt probles arguments Alexis Berlemont
2016-11-16 23:56 ` [PATCH 2/2] perf probe: add sdt probes arguments into the uprobe cmd string Alexis Berlemont
2016-11-17 9:04 ` Hemant Kumar
2016-11-18 23:56 ` [PATCH v2 0/2] " Alexis Berlemont
2016-11-18 23:56 ` [PATCH v2 1/2] perf sdt: add scanning of sdt probles arguments Alexis Berlemont
2016-11-25 14:40 ` Arnaldo Carvalho de Melo
2016-11-26 0:58 ` [PATCH v4 0/2] perf probe: add sdt probes arguments into the uprobe cmd string Alexis Berlemont
2016-12-05 23:42 ` Alexis Berlemont
2016-12-06 14:45 ` Arnaldo Carvalho de Melo
2016-11-26 0:58 ` [PATCH v4 1/2] perf sdt: add scanning of sdt probles arguments Alexis Berlemont
2016-12-07 2:44 ` Masami Hiramatsu
2016-11-26 0:58 ` [PATCH v4 2/2] perf probe: add sdt probes arguments into the uprobe cmd string Alexis Berlemont
2016-12-07 3:26 ` Masami Hiramatsu
2016-12-09 15:14 ` Arnaldo Carvalho de Melo
2016-12-10 10:00 ` Masami Hiramatsu
2016-12-14 0:07 ` [PATCH v5 0/2] " Alexis Berlemont
2016-12-14 7:36 ` Ingo Molnar
2017-01-23 11:23 ` Ravi Bangoria
2017-02-22 22:41 ` Alexis Berlemont
2017-01-24 6:58 ` Ravi Bangoria
2017-01-24 8:22 ` Ingo Molnar
2017-01-24 8:36 ` Ravi Bangoria
2017-02-02 11:11 ` [PATCH 0/5] perf/sdt: Argument support for x86 and powepc Ravi Bangoria
2017-02-02 11:11 ` [PATCH 1/5] perf/sdt: Show proper hint Ravi Bangoria
2017-02-02 13:40 ` Ingo Molnar
2017-02-02 16:20 ` Arnaldo Carvalho de Melo
2017-02-03 10:26 ` [PATCH v2] " Ravi Bangoria
2017-02-03 15:18 ` Arnaldo Carvalho de Melo
2017-02-07 7:53 ` Ingo Molnar
2017-02-07 15:50 ` Arnaldo Carvalho de Melo
2017-02-07 8:00 ` Ingo Molnar
2017-02-16 10:16 ` [RFC] perf/sdt: Directly record SDT event with 'perf record' Ravi Bangoria
2017-02-20 7:08 ` Ingo Molnar
2017-02-20 8:21 ` Ravi Bangoria
2017-02-20 8:42 ` Ingo Molnar
2017-02-20 11:01 ` Ravi Bangoria [this message]
2017-02-20 14:11 ` Arnaldo Carvalho de Melo
2017-02-23 8:13 ` Ravi Bangoria
2017-02-23 12:48 ` Arnaldo Carvalho de Melo
2017-02-07 1:13 ` [PATCH v2] perf/sdt: Show proper hint Masami Hiramatsu
2017-02-10 7:44 ` [tip:perf/core] perf sdt: Show proper hint when event not yet in place via 'perf probe' tip-bot for Ravi Bangoria
2017-02-02 11:11 ` [PATCH 2/5] perf/sdt/x86: Add renaming logic for rNN and other registers Ravi Bangoria
2017-02-07 3:11 ` Masami Hiramatsu
2017-03-21 14:08 ` Arnaldo Carvalho de Melo
2017-03-24 18:45 ` [tip:perf/core] perf sdt x86: " tip-bot for Ravi Bangoria
2017-02-02 11:11 ` [PATCH 3/5] perf/sdt/x86: Move OP parser to tools/perf/arch/x86/ Ravi Bangoria
2017-02-07 3:11 ` Masami Hiramatsu
2017-02-07 5:22 ` Ravi Bangoria
2017-03-21 14:10 ` Arnaldo Carvalho de Melo
2017-03-21 23:00 ` Masami Hiramatsu
2017-03-22 11:22 ` Arnaldo Carvalho de Melo
2017-03-21 14:55 ` Masami Hiramatsu
2017-02-02 11:11 ` [PATCH 4/5] perf/sdt/powerpc: Add argument support Ravi Bangoria
2017-02-02 11:11 ` [PATCH 5/5] perf/probe: Change MAX_CMDLEN Ravi Bangoria
2017-02-07 1:40 ` Masami Hiramatsu
2017-02-07 5:45 ` [PATCH v2] " Ravi Bangoria
2017-03-21 5:19 ` Masami Hiramatsu
2017-03-21 13:37 ` Arnaldo Carvalho de Melo
2017-03-24 18:43 ` [tip:perf/core] perf probe: " tip-bot for Ravi Bangoria
2017-02-07 2:55 ` [PATCH 0/5] perf/sdt: Argument support for x86 and powepc Masami Hiramatsu
2017-03-06 7:53 ` Ravi Bangoria
2017-03-06 13:42 ` Masami Hiramatsu
2017-03-21 5:08 ` [PATCH v5 0/2] perf probe: add sdt probes arguments into the uprobe cmd string Masami Hiramatsu
2016-12-14 0:07 ` [PATCH v5 1/2] perf sdt: add scanning of sdt probles arguments Alexis Berlemont
2017-03-06 13:39 ` Masami Hiramatsu
2017-03-21 13:52 ` Arnaldo Carvalho de Melo
2017-03-24 18:44 ` [tip:perf/core] perf sdt: Add scanning of sdt probes arguments tip-bot for Alexis Berlemont
2016-12-14 0:07 ` [PATCH v5 2/2] perf probe: add sdt probes arguments into the uprobe cmd string Alexis Berlemont
2017-01-24 8:50 ` Ravi Bangoria
2017-03-06 17:23 ` Masami Hiramatsu
2017-03-24 18:44 ` [tip:perf/core] perf probe: Add " tip-bot for Alexis Berlemont
2016-11-18 23:56 ` [PATCH v2 2/2] perf probe: add " Alexis Berlemont
2016-11-21 10:25 ` Hemant Kumar
2016-11-24 23:13 ` [PATCH v3 0/2] " Alexis Berlemont
2016-11-24 23:13 ` [PATCH v3 1/2] perf sdt: add scanning of sdt probles arguments Alexis Berlemont
2016-11-24 23:13 ` [PATCH v3 2/2] perf probe: add sdt probes arguments into the uprobe cmd string Alexis Berlemont
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=58AACC9E.8070009@linux.vnet.ibm.com \
--to=ravi.bangoria@linux.vnet.ibm.com \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=ak@linux.intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=ananth@in.ibm.com \
--cc=brendan.d.gregg@gmail.com \
--cc=colin.ing@canonical.com \
--cc=hekuang@huawei.com \
--cc=hemant@linux.vnet.ibm.com \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=masami.hiramatsu.pt@hitachi.com \
--cc=mathieu.poirier@linaro.org \
--cc=mingo@kernel.org \
--cc=mingo@redhat.com \
--cc=naveen.n.rao@linux.vnet.ibm.com \
--cc=peterz@infradead.org \
--cc=sukadev@linux.vnet.ibm.com \
--cc=treeze.taeung@gmail.com \
--cc=wangnan0@huawei.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.