On 2015/07/21 1:20, Namhyung Kim wrote: > Hi Brendan, > > On Sun, Jul 19, 2015 at 10:47:31PM -0700, Brendan Gregg wrote: >> G'Day Masami-san, Namhyung, >> >> I'm really looking forward to this feature -- very useful, thanks!... >> >> On Sat, Jul 18, 2015 at 9:24 PM, Namhyung Kim <namhy...@kernel.org> wrote: >>> Hi Masami, >>> >>> On Fri, Jul 17, 2015 at 12:21:42PM +0900, Masami Hiramatsu wrote: >>>> Now I'm thinking that we should avoid using %event syntax for perf-list >>>> and perf-record to avoid confusion. For example, suppose that we have >>>> "libfoo:bar" SDT event, when we just scanned the libfoo binary and >>>> use it via perf-record, we'll run perf record -e "%libfoo:bar". >>>> However, after we set the probe via perf-probe, we have to run >>>> perf record -e "libfoo:bar". That difference looks no good. >>>> So, I think in both case it should accept -e "libfoo:bar" syntax. >>> >>> I don't remember how the SDT events should be shown to users. Sorry >>> if I'm missing something here. >>> >>> AFAIK an SDT event consists of a provider and an event name. So it >>> can be simply 'provider:event' like tracepoints or >>> 'binary:provider_event' like uprobes. >>> >>> I like the former because it's simpler but it needs to guarantee that >>> it doesn't clash with existing tracepoints/[ku]probes. So IIUC we >>> chose the '%' sign to distinguish them. But after setting a probe at >>> it, the group name should be the binary name. So the whole event name >>> might be changed, and this is not good. >> >> I don't think we should worry about the clash, as the provider name >> should differentiate. > > But there's no guarantee. Maybe an userspace tool which deals with a > kernel module has SDT names as same as the kernel module's tracepoint > names. It might or might not be a problem if we can handle those > duplicate names somehow.
I'd like to suggest to choose the behavior on scanning SDT. Since the perf-probe just relays on what the event names are stored on the cache file, we can choose "sdt_" prefix or not when scanning the SDT. If the name is already used by the kernel tracepoint, we can add sdt_ prefix or some sort of suffix. Thank you, >> So I think "libfoo:bar" with perf record is >> better. After adding them to the cache (via % if needed), I'd think >> they would be best looking like tracepoints. Eg, listing them together >> they can be differentiated, something like: >> >> # perf list >> [...] >> block:block_rq_abort [Tracepoint event] >> block:block_rq_requeue [Tracepoint event] >> block:block_rq_complete [Tracepoint event] >> [...] >> libc:memory_heap_new [User tracepoint event] >> libc:memory_heap_free [User tracepoint event] >> libc:memory_heap_more [User tracepoint event] >> [...] >> >> Then used the same. > > Yes, as I said I also prefer this simpler form. Maybe we can choose > to use another names for low-level plumbing inside the perf tools, but > I still think that users should be able to use simple names like above. -- Masami HIRAMATSU Linux Technology Research Center, System Productivity Research Dept. Center for Technology Innovation - Systems Engineering Hitachi, Ltd., Research & Development Group E-mail: masami.hiramatsu...@hitachi.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
