public inbox for systemtap@sourceware.org
 help / color / mirror / Atom feed
From: Josh Stone <jistone@redhat.com>
To: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>,
	       Hemant Kumar <hemant@linux.vnet.ibm.com>,
	linux-kernel@vger.kernel.org,        srikar@linux.vnet.ibm.com,
	peterz@infradead.org, oleg@redhat.com,
	       hegdevasant@linux.vnet.ibm.com, mingo@redhat.com,
	       systemtap@sourceware.org, aravinda@linux.vnet.ibm.com,
	penberg@iki.fi,        Arnaldo Carvalho de Melo <acme@redhat.com>
Subject: Re: [PATCH v4 5/5] perf/sdt: Add support to perf record to trace SDT events
Date: Thu, 06 Nov 2014 02:16:00 -0000	[thread overview]
Message-ID: <545AD9C7.50205@redhat.com> (raw)
In-Reply-To: <5459E865.6050207@hitachi.com>

On 11/05/2014 01:05 AM, Masami Hiramatsu wrote:
> [Off topic] I really don't like that the current SDT's semaphore. If the user apps
> see the instruction at the probe point, it is easy to check whether the event is
> enabled or not. Thus I recommend to change its implementation and update version
> instead of supporting current semaphore by perftools.

You and I have banged heads on this before, but I don't think checking
the instruction is a simple as you seem to think.  I invite you to
prototype this, and if you get it working we can discuss the tradeoffs.

The good news is that other tools (stap and gdb) won't need to care.  If
the SDT semaphore goes automatic, then we can just set that note field
to zero, unused from the tool's perspective.

Another tactic is to just discourage developers from using the semaphore
in the first place, as it's a completely optional feature.  The marker
is just a NOP, so adding some "if (enabled) {...}" around it is often a
useless load and branch.  It does make sense if the probe wants to
provide some expensively-computed arguments though, like cpython does to
prepare a function name string.  So if you see a project testing the
semaphore around simple arguments, I'd suggest they just probe directly
instead.

Thanks,
Josh

  reply	other threads:[~2014-11-06  2:16 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-02 12:40 [PATCH v4 0/5] perf/sdt: SDT events listing/probing Hemant Kumar
2014-11-02 12:41 ` [PATCH v4 1/5] perf/sdt: ELF support for SDT Hemant Kumar
2014-11-02 12:42 ` [PATCH v4 2/5] perf/sdt: Add SDT events into a cache Hemant Kumar
2014-11-02 12:42 ` [PATCH v4 3/5] perf/sdt: Show SDT cache contents Hemant Kumar
2014-11-02 12:42 ` [PATCH v4 4/5] perf/sdt: Delete SDT events from cache Hemant Kumar
2014-11-02 12:43 ` [PATCH v4 5/5] perf/sdt: Add support to perf record to trace SDT events Hemant Kumar
2014-11-04  7:38   ` Namhyung Kim
2014-11-04  8:06     ` Hemant Kumar
2014-11-04 12:57       ` Masami Hiramatsu
     [not found]         ` <5459BD3E.7010804@linux.vnet.ibm.com>
2014-11-05  6:51           ` Hemant Kumar
2014-11-05  9:07             ` Masami Hiramatsu
2014-11-05 13:28               ` Arnaldo Carvalho de Melo
2014-11-05  7:06         ` Namhyung Kim
2014-11-05  9:05           ` Masami Hiramatsu
2014-11-06  2:16             ` Josh Stone [this message]
2014-11-06  5:34               ` Masami Hiramatsu
2014-11-06  7:06             ` Hemant Kumar
2014-11-06 14:56               ` Masami Hiramatsu
2014-11-07  8:21               ` [RFC] perf-cache command interface design Masami Hiramatsu
2014-11-07  8:42                 ` Peter Zijlstra
2014-11-07 13:58                   ` [PATCH RESEND 1/2] perf tools: Move disable_buildid_cache() to util/build-id.c Namhyung Kim
2014-11-07 13:58                     ` [PATCH 2/2] perf tools: Add record.use-buildid-cache config option Namhyung Kim
2014-11-07 15:16                   ` [RFC] perf-cache command interface design David Ahern
2014-11-07 15:33                     ` Arnaldo Carvalho de Melo
2014-11-07 10:51                 ` Hemant Kumar
2014-11-08  4:15                   ` Masami Hiramatsu
2014-11-07 14:38                 ` Arnaldo Carvalho de Melo
2014-11-08  4:26                   ` Masami Hiramatsu
2014-11-07 14:43                 ` Namhyung Kim
2014-11-08  4:38                   ` Masami Hiramatsu
2014-11-10 10:59                 ` Masami Hiramatsu
2014-11-10 12:23                   ` Arnaldo Carvalho de Melo
2014-11-11  6:53                     ` Masami Hiramatsu
2014-11-11 13:10                       ` Arnaldo Carvalho de Melo
2014-11-12 15:26                         ` Masami Hiramatsu
2014-11-17  3:09                           ` Namhyung Kim
2014-11-17  3:17                             ` Masami Hiramatsu
2014-11-17 22:09                               ` Masami Hiramatsu
2014-11-18  4:51                                 ` Namhyung Kim
2014-11-18 11:16                                   ` Masami Hiramatsu
2014-11-18  4:41                               ` Namhyung Kim
2014-11-18 10:32                                 ` Masami Hiramatsu
2014-11-17 18:59                             ` Arnaldo Carvalho de Melo
2014-11-18  4:46                               ` Namhyung Kim
2014-11-10 12:05                 ` Hagen Paul Pfeifer
2014-11-10 12:31                   ` Arnaldo Carvalho de Melo
2014-11-10 12:50                   ` Peter Zijlstra
2014-11-10 13:37                     ` Hagen Paul Pfeifer
2014-11-05 18:24           ` [PATCH v4 5/5] perf/sdt: Add support to perf record to trace SDT events Hemant Kumar

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=545AD9C7.50205@redhat.com \
    --to=jistone@redhat.com \
    --cc=acme@redhat.com \
    --cc=aravinda@linux.vnet.ibm.com \
    --cc=hegdevasant@linux.vnet.ibm.com \
    --cc=hemant@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masami.hiramatsu.pt@hitachi.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=oleg@redhat.com \
    --cc=penberg@iki.fi \
    --cc=peterz@infradead.org \
    --cc=srikar@linux.vnet.ibm.com \
    --cc=systemtap@sourceware.org \
    /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 read-only IMAP folder(s) and NNTP newsgroup(s).