dle-develop — Dependable Linux Effort mailing list for developers

Re: [Dle-develop] [PATCH -tip 0/8] perf-probe updates

[Arnaldo C. de M. <ac...@re...>]

Em Fri, Dec 11, 2009 at 04:15:30PM -0500, Masami Hiramatsu escreveu:
> Arnaldo Carvalho de Melo wrote:
> > Em Wed, Dec 09, 2009 at 04:41:39PM -0500, Masami Hiramatsu escreveu:
> >> Masami Hiramatsu wrote:
> >>>> Here the failure was because the symbol does not exist.
> >>>
> >>> Yeah, so that's what I'm expecting to be implemented with below item :-)
> >>>
> >>>    - Symbol search by libelf/kallsyms
> >>>
> >>> I guess it will be done by using symbol.c.
> >>> (Actually, current
> >>
> >> Oops, I might miss something, forgot it.
> >> actually, current perf probe doesn't decode kallsyms/elf, it just depends on
> >> kprobe-tracer. And kprobe-tracer will return -EINVAL not only if the symbol
> >> was not found, but also other reasons.
> >>
> >> So, if we want to show below message, we need to decode elf or kallsyms.
> > 
> > Should be easy now with my latest patch :-)
> 
> Thanks! that's looks very useful for me!

Please let me know of any shortcomings you may find!

- Arnaldo

Re: [Dle-develop] [PATCH -tip 0/8] perf-probe updates

[Masami H. <mhi...@re...>]

Arnaldo Carvalho de Melo wrote:
> Em Wed, Dec 09, 2009 at 04:41:39PM -0500, Masami Hiramatsu escreveu:
>> Masami Hiramatsu wrote:
>>>> Here the failure was because the symbol does not exist.
>>>
>>> Yeah, so that's what I'm expecting to be implemented with below item :-)
>>>
>>>    - Symbol search by libelf/kallsyms
>>>
>>> I guess it will be done by using symbol.c.
>>> (Actually, current
>>
>> Oops, I might miss something, forgot it.
>> actually, current perf probe doesn't decode kallsyms/elf, it just depends on
>> kprobe-tracer. And kprobe-tracer will return -EINVAL not only if the symbol
>> was not found, but also other reasons.
>>
>> So, if we want to show below message, we need to decode elf or kallsyms.
> 
> Should be easy now with my latest patch :-)

Thanks! that's looks very useful for me!


-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhi...@re...

Re: [Dle-develop] [PATCH -tip 0/8] perf-probe updates

[Arnaldo C. de M. <ac...@re...>]

Em Wed, Dec 09, 2009 at 04:41:39PM -0500, Masami Hiramatsu escreveu:
> Masami Hiramatsu wrote:
> >> Here the failure was because the symbol does not exist.
> > 
> > Yeah, so that's what I'm expecting to be implemented with below item :-)
> > 
> >    - Symbol search by libelf/kallsyms
> > 
> > I guess it will be done by using symbol.c.
> > (Actually, current
> 
> Oops, I might miss something, forgot it.
> actually, current perf probe doesn't decode kallsyms/elf, it just depends on
> kprobe-tracer. And kprobe-tracer will return -EINVAL not only if the symbol
> was not found, but also other reasons.
> 
> So, if we want to show below message, we need to decode elf or kallsyms.

Should be easy now with my latest patch :-)
 
> > >
> > > There's similar problems in most other failure cases. Trying to remove a
> > > non-existent probe gives:
> > >
> > >   # perf probe -d test
> > >   Warning: event "probe:test" is not found.

Re: [Dle-develop] [PATCH v4] [RFC] tracepoint: Add signal coredump tracepoint

[KOSAKI M. <kos...@jp...>]

> Add signal coredump tracepoint which shows signal number,
> mm->flags, core file size limitation, the result of
> coredump, and core file name.
> 
> This tracepoint requirement comes mainly from the viewpoint of
> administrators. Since now we have introduced many coredump
> configurations (e.g. dumpable, coredump_filter, core_pattern,
> etc) and some of them can be modified by users, it will be hard
> to know what was actually dumped (or not dumped) after some
> problem happened on the system. For example, a process didn't
> generated core, coredump doesn't have some sections, etc.
> In those cases, the coredump tracepoint can help us to know
> why the core file is so big or small, or not generated, by
> recording all configurations for all processes on the system.
> That will reduce system-administration cost.
> 
> Changes in v4:
>  - Rename limit trace-argument to core_size_limit, because
>    of user friendly output.

Looks good to me.

Re: [Dle-develop] [PATCH -tip 0/8] perf-probe updates

[Masami H. <mhi...@re...>]

Masami Hiramatsu wrote:
>> Here the failure was because the symbol does not exist.
> 
> Yeah, so that's what I'm expecting to be implemented with below item :-)
> 
>    - Symbol search by libelf/kallsyms
> 
> I guess it will be done by using symbol.c.
> (Actually, current

Oops, I might miss something, forgot it.
actually, current perf probe doesn't decode kallsyms/elf, it just depends on
kprobe-tracer. And kprobe-tracer will return -EINVAL not only if the symbol
was not found, but also other reasons.

So, if we want to show below message, we need to decode elf or kallsyms.

> >
> > There's similar problems in most other failure cases. Trying to remove a
> > non-existent probe gives:
> >
> >   # perf probe -d test
> >   Warning: event "probe:test" is not found.
> >

Thank you,

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhi...@re...

[Dle-develop] [PATCH v4] [RFC] tracepoint: Add signal coredump tracepoint

[Masami H. <mhi...@re...>]

Add signal coredump tracepoint which shows signal number,
mm->flags, core file size limitation, the result of
coredump, and core file name.

This tracepoint requirement comes mainly from the viewpoint of
administrators. Since now we have introduced many coredump
configurations (e.g. dumpable, coredump_filter, core_pattern,
etc) and some of them can be modified by users, it will be hard
to know what was actually dumped (or not dumped) after some
problem happened on the system. For example, a process didn't
generated core, coredump doesn't have some sections, etc.
In those cases, the coredump tracepoint can help us to know
why the core file is so big or small, or not generated, by
recording all configurations for all processes on the system.
That will reduce system-administration cost.

Changes in v4:
 - Rename limit trace-argument to core_size_limit, because
   of user friendly output.

Changes in v3:
 - Move tracepoint at the end of do_coredump() for tracing
   the result of coredump.
 - Use retval to record error-code at every failure points
   for passing the result of coredump to tracepoint.
 - Trace retval instead of cprm->file for recording the
   result of coredump.

Changes in v2:
 - Fix a bug to clear file local variable when
   call_usermodehelper_pipe() is failed.

Signed-off-by: Masami Hiramatsu <mhi...@re...>
Cc: Oleg Nesterov <ol...@re...>
Cc: Roland McGrath <ro...@re...>
Cc: Jason Baron <jb...@re...>
Cc: Ingo Molnar <mi...@el...>
Cc: Andrew Morton <ak...@li...>
Cc: KOSAKI Motohiro <kos...@jp...>
---

 fs/exec.c                     |   58 ++++++++++++++++++++++++++++++-----------
 include/trace/events/signal.h |   48 ++++++++++++++++++++++++++++++++++
 2 files changed, 90 insertions(+), 16 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 0a5d944..d67ed5a 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -55,6 +55,7 @@
 #include <linux/fsnotify.h>
 #include <linux/fs_struct.h>
 #include <linux/pipe_fs_i.h>
+#include <trace/events/signal.h>
 
 #include <asm/uaccess.h>
 #include <asm/mmu_context.h>
@@ -1753,7 +1754,7 @@ static void wait_for_dump_helpers(struct file *file)
 void do_coredump(long signr, int exit_code, struct pt_regs *regs)
 {
 	struct core_state core_state;
-	char corename[CORENAME_MAX_SIZE + 1];
+	char corename[CORENAME_MAX_SIZE + 1] = "";
 	struct mm_struct *mm = current->mm;
 	struct linux_binfmt * binfmt;
 	struct inode * inode;
@@ -1768,6 +1769,7 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs)
 	static atomic_t core_dump_count = ATOMIC_INIT(0);
 	struct coredump_params cprm = {
 		.signr = signr,
+		.file = NULL,
 		.regs = regs,
 		.limit = current->signal->rlim[RLIMIT_CORE].rlim_cur,
 		/*
@@ -1781,8 +1783,10 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs)
 	audit_core_dumps(signr);
 
 	binfmt = mm->binfmt;
-	if (!binfmt || !binfmt->core_dump)
+	if (!binfmt || !binfmt->core_dump) {
+		retval = -ENOSYS;
 		goto fail;
+	}
 
 	cred = prepare_creds();
 	if (!cred) {
@@ -1795,6 +1799,7 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs)
 	 * If another thread got here first, or we are not dumpable, bail out.
 	 */
 	if (mm->core_state || !__get_dumpable(cprm.mm_flags)) {
+		/* This is not an error. retval should be 0 */
 		up_write(&mm->mmap_sem);
 		put_cred(cred);
 		goto fail;
@@ -1833,11 +1838,14 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs)
 	ispipe = format_corename(corename, signr);
 	unlock_kernel();
 
-	if ((!ispipe) && (cprm.limit < binfmt->min_coredump))
+	if ((!ispipe) && (cprm.limit < binfmt->min_coredump)) {
+		retval = -EFBIG;
 		goto fail_unlock;
+	}
 
  	if (ispipe) {
 		if (cprm.limit == 0) {
+			retval = -EINVAL;
 			/*
 			 * Normally core limits are irrelevant to pipes, since
 			 * we're not writing to the file system, but we use
@@ -1861,6 +1869,7 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs)
 
 		dump_count = atomic_inc_return(&core_dump_count);
 		if (core_pipe_limit && (core_pipe_limit < dump_count)) {
+			retval = -EFBIG;
 			printk(KERN_WARNING "Pid %d(%s) over core_pipe_limit\n",
 			       task_tgid_vnr(current), current->comm);
 			printk(KERN_WARNING "Skipping core dump\n");
@@ -1869,6 +1878,7 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs)
 
 		helper_argv = argv_split(GFP_KERNEL, corename+1, &helper_argc);
 		if (!helper_argv) {
+			retval = -ENOMEM;
 			printk(KERN_WARNING "%s failed to allocate memory\n",
 			       __func__);
 			goto fail_dropcount;
@@ -1877,8 +1887,9 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs)
 		cprm.limit = RLIM_INFINITY;
 
 		/* SIGPIPE can happen, but it's just never processed */
-		if (call_usermodehelper_pipe(helper_argv[0], helper_argv, NULL,
-				&cprm.file)) {
+		retval = call_usermodehelper_pipe(helper_argv[0], helper_argv,
+						  NULL, &cprm.file);
+		if (retval < 0) {
  			printk(KERN_INFO "Core dump to %s pipe failed\n",
 			       corename);
 			goto fail_dropcount;
@@ -1887,31 +1898,43 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs)
 		cprm.file = filp_open(corename,
 				 O_CREAT | 2 | O_NOFOLLOW | O_LARGEFILE | flag,
 				 0600);
-	if (IS_ERR(cprm.file))
+	if (IS_ERR(cprm.file)) {
+		retval = (int)PTR_ERR(cprm.file);
 		goto fail_dropcount;
+	}
 	inode = cprm.file->f_path.dentry->d_inode;
-	if (inode->i_nlink > 1)
+	if (inode->i_nlink > 1) {
+		retval = -EMLINK;
 		goto close_fail;	/* multiple links - don't dump */
-	if (!ispipe && d_unhashed(cprm.file->f_path.dentry))
+	}
+	if (!ispipe && d_unhashed(cprm.file->f_path.dentry)) {
+		retval = -EBADF;
 		goto close_fail;
+	}
 
 	/* AK: actually i see no reason to not allow this for named pipes etc.,
 	   but keep the previous behaviour for now. */
-	if (!ispipe && !S_ISREG(inode->i_mode))
+	if (!ispipe && !S_ISREG(inode->i_mode)) {
+		retval = -EBADF;
 		goto close_fail;
+	}
 	/*
 	 * Dont allow local users get cute and trick others to coredump
 	 * into their pre-created files:
 	 */
-	if (inode->i_uid != current_fsuid())
+	if (inode->i_uid != current_fsuid()) {
+		retval = -EPERM;
 		goto close_fail;
-	if (!cprm.file->f_op)
-		goto close_fail;
-	if (!cprm.file->f_op->write)
-		goto close_fail;
-	if (!ispipe &&
-	    do_truncate(cprm.file->f_path.dentry, 0, 0, cprm.file) != 0)
+	}
+	if (!cprm.file->f_op || !cprm.file->f_op->write) {
+		retval = -EINVAL;
 		goto close_fail;
+	}
+	if (!ispipe) {
+		retval = do_truncate(cprm.file->f_path.dentry, 0, 0, cprm.file);
+		if (retval != 0)
+			goto close_fail;
+	}
 
 	retval = binfmt->core_dump(&cprm);
 
@@ -1932,5 +1955,8 @@ fail_unlock:
 	put_cred(cred);
 	coredump_finish(mm);
 fail:
+	/* Trace coredump parameters and return value */
+	trace_signal_coredump(&cprm, corename, retval);
+
 	return;
 }
diff --git a/include/trace/events/signal.h b/include/trace/events/signal.h
index a510b75..6dbc856 100644
--- a/include/trace/events/signal.h
+++ b/include/trace/events/signal.h
@@ -4,8 +4,10 @@
 #if !defined(_TRACE_SIGNAL_H) || defined(TRACE_HEADER_MULTI_READ)
 #define _TRACE_SIGNAL_H
 
+#include <linux/err.h>
 #include <linux/signal.h>
 #include <linux/sched.h>
+#include <linux/binfmts.h>
 #include <linux/tracepoint.h>
 
 #define TP_STORE_SIGINFO(__entry, info)				\
@@ -167,6 +169,52 @@ TRACE_EVENT(signal_lose_info,
 	TP_printk("sig=%d group=%d errno=%d code=%d",
 		  __entry->sig, __entry->group, __entry->errno, __entry->code)
 );
+
+/**
+ * signal_coredump - called when dumping core by signal
+ * @cprm: pointer to struct coredump_params
+ * @core_name: core-name string
+ * @retval: return value of binfmt->coredump or error-code
+ *
+ * Current process dumps core file to 'core_name' file, because 'cprm->signr'
+ * signal is delivered.
+ * 'retval' is an error code or 0/1. retval == 1 means the core file was
+ * dumped successfully and retval == 0 means binfmt->coredump failed to dump.
+ * If retval < 0, this means do_coredump() failed to dump core file before
+ * calling binfmt->coredump.
+ */
+TRACE_EVENT(signal_coredump,
+
+	TP_PROTO(struct coredump_params *cprm, const char *core_name,
+		 int retval),
+
+	TP_ARGS(cprm, core_name, retval),
+
+	TP_STRUCT__entry(
+		__field(	int,		sig		)
+		__field(	unsigned long,	core_size_limit	)
+		__field(	unsigned long,	flags		)
+		__field(	int,		retval		)
+		__string(	name,		core_name	)
+	),
+
+
+	TP_fast_assign(
+		__entry->sig			= (int)cprm->signr;
+		__entry->core_size_limit	= cprm->limit;
+		__entry->flags			= cprm->mm_flags;
+		__entry->retval			= retval;
+		__assign_str(name,		core_name);
+	),
+
+	TP_printk("sig=%d core_size_limit=%lu dumpable=0x%lx dump_filter=0x%lx"
+		  " corename=\"%s\" retval=%d",
+		  __entry->sig, __entry->core_size_limit,
+		  __entry->flags & MMF_DUMPABLE_MASK,
+		  (__entry->flags & MMF_DUMP_FILTER_MASK) >>
+		  MMF_DUMP_FILTER_SHIFT,
+		  __get_str(name), __entry->retval)
+);
 #endif /* _TRACE_SIGNAL_H */
 
 /* This part must be outside protection */


-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhi...@re...

Re: [Dle-develop] [PATCH -tip 0/8] perf-probe updates

[Masami H. <mhi...@re...>]

Hi Ingo,

Thank you for error reporting :-)

Ingo Molnar wrote:
> There's another small hickup i had - when i typoed 'perf probe -', it
> gave me:
>
>   # perf probe -
>   No dwarf info found in the vmlinux - please rebuild with CONFIG_DEBUG_INFO.
>   An error occurred in debuginfo analysis. Try to use symbols.
>     Fatal: Failed to write event: Invalid argument

OK, perhaps, you are expecting "perf probe -" works similar to "perf record -",
but that's not implemented yet...
Anyway, I think "perf probe -" should show a usage and return.


> Similar thing happens if i try to probe a non-existent symbol:
>
>   # perf probe test
>   No dwarf info found in the vmlinux - please rebuild with CONFIG_DEBUG_INFO.
>   An error occurred in debuginfo analysis. Try to use symbols.
>     Fatal: Failed to write event: Invalid argument
>
> I think we should print something more helpful, such as:
>
>   # perf probe test
>   Fatal: Kernel symbol 'test' not found - probe not added.
>
> the debuginfo printout is not helpful in this case - we should fall back
> to symbols silently, unless the nature of the error indicates that we
> fail _because_ there's no debuginfo.

OK.

> Here the failure was because the symbol does not exist.

Yeah, so that's what I'm expecting to be implemented with below item :-)

   - Symbol search by libelf/kallsyms

I guess it will be done by using symbol.c.
(Actually, current

781 221 7109

>
> There's similar problems in most other failure cases. Trying to remove a
> non-existent probe gives:
>
>   # perf probe -d test
>   Warning: event "probe:test" is not found.
>
> It should say something like:
>
>   # perf probe -d test
>   Info: event "probe:test" does not exist, could not remove it.

Sure.

> Also, it's possible to add multiple probes to the same function, using
> 'perf probe schedule' + 'perf probe schedule', etc. While in general it
> makes sense to allow it, by default we should refuse the second,
> identical probe on the symbol - and add a -f/--force option to force
> duplicate probes. I.e. the second probe should print:
>
>   # perf probe schedule
>   Info: event "probe:schedule" already exists. (Use -f to force a duplicate.)
>
> etc.

Hmm, if you mean 'refusing the second issued command',
it might be good, because it can avoid to add events with
unwilling names.

However, I don't think it's so useful generally, because
some code lines can be expanded to several addresses.
In that case, we need to add several events at once,
and each event has different names.
e.g.

# ./perf probe schedule:10
Added new event:
   probe:schedule                           (on schedule+1)
Added new event:
   probe:schedule_1                         (on schedule+19)
Added new event:
   probe:schedule_2                         (on schedule+19)
Added new event:
   probe:schedule_3                         (on schedule+28)
Added new event:
   probe:schedule_4                         (on schedule+38)

You can now use it on all perf tools, such as:

         perf record -e probe:schedule_4 -a sleep 1
---
Same things happens when probing inlined functions.

I guess this issue might be solved by below items;
   - Support glob expression with --del option (like --del "*")
   - Support event/group name specifying for new events

e.g.
Adding new events with another name.

# perf probe --add mygroup:sched_enter=schedule
Added new event:
   mygroup:sched_enter                      (on schedule+0)

# perf probe --add mygroup:sched_event=schedule:10
Added new event:
   mygroup:sched_event                      (on schedule+1)
Added new event:
   mygroup:sched_event_1                    (on schedule+19)
Added new event:
   mygroup:sched_event_2                    (on schedule+19)
Added new event:
   mygroup:sched_event_3                    (on schedule+28)
Added new event:
   mygroup:sched_event_4                    (on schedule+38)

And record it by glob specifying.

# perf record -e mygroup:sched_event* -a sleep 1

or

# perf record -e mygroup:sched_enter* -a sleep 1

What would you think about it?

> Please try out various sensible and also less sensible options of
> this tool and try to make it break - and see whether the behavior is
> intuitive and obvious to users - whether the messages are consistent,
> etc. etc.

OK, actually, perf-probe needs to be brushed up. Any comments, reports
and opinions are welcome. I'd like to update todo list, based on what
is useful and which is important for users.

Thank you,

>
> 	Ingo

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhi...@re...

Re: [Dle-develop] [PATCH v3] [RFC] tracepoint: Add signal coredump tracepoint

[Masami H. <mhi...@re...>]

Masami Hiramatsu wrote:
>
>
> KOSAKI Motohiro wrote:
>>> +	TP_fast_assign(
>>> +		__entry->sig	= (int)cprm->signr;
>>> +		__entry->limit	= cprm->limit;
>>> +		__entry->flags	= cprm->mm_flags;
>>> +		__entry->retval	= retval;
>>> +		__assign_str(name,	core_name);
>>> +	),
>>> +
>>> +	TP_printk("sig=%d limit=%lu dumpable=0x%lx dump_filter=0x%lx "
>>> +		  "corename=\"%s\" retval=%d",
>>> +		  __entry->sig, __entry->limit,
>>> +		  __entry->flags&   MMF_DUMPABLE_MASK,
>>> +		  (__entry->flags&   MMF_DUMP_FILTER_MASK)>>
>>> +		  MMF_DUMP_FILTER_SHIFT,
>>> +		  __get_str(name), __entry->retval)
>>> +);
>>>    #endif /* _TRACE_SIGNAL_H */
>>
>> I don't think "limit" is userfriendly name, core_limit or core_size_limit is better?
>> plus, we have core_pipe_limit sysctl too. (it's similar but different concept limit).
>
> Ah, I missed it. OK, so I'll rename core_limit and add core_pipe_limit.

Hmm, perhaps, would we need a pair of core_pipe_limit and dump_count?
because it limits the number of concurrently dump-to-pipe and the number
is stored on dump_count.
Or, maybe it is enough to trace current parameters, because if we hit the
core_pipe_limit, we can see -EFBIG at retval parameter.


> Thank you for pointed it out :-)
>
>>
>> other parts looks good to me.
>>
>>
>>
>
> Thanks!
>

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhi...@re...

Re: [Dle-develop] [PATCH v3] [RFC] tracepoint: Add signal coredump tracepoint

[Masami H. <mhi...@re...>]

KOSAKI Motohiro wrote:
>> +	TP_fast_assign(
>> +		__entry->sig	= (int)cprm->signr;
>> +		__entry->limit	= cprm->limit;
>> +		__entry->flags	= cprm->mm_flags;
>> +		__entry->retval	= retval;
>> +		__assign_str(name,	core_name);
>> +	),
>> +
>> +	TP_printk("sig=%d limit=%lu dumpable=0x%lx dump_filter=0x%lx "
>> +		  "corename=\"%s\" retval=%d",
>> +		  __entry->sig, __entry->limit,
>> +		  __entry->flags&  MMF_DUMPABLE_MASK,
>> +		  (__entry->flags&  MMF_DUMP_FILTER_MASK)>>
>> +		  MMF_DUMP_FILTER_SHIFT,
>> +		  __get_str(name), __entry->retval)
>> +);
>>   #endif /* _TRACE_SIGNAL_H */
>
> I don't think "limit" is userfriendly name, core_limit or core_size_limit is better?
> plus, we have core_pipe_limit sysctl too. (it's similar but different concept limit).

Ah, I missed it. OK, so I'll rename core_limit and add core_pipe_limit.
Thank you for pointed it out :-)

>
> other parts looks good to me.
>
>
>

Thanks!

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhi...@re...

Re: [Dle-develop] [PATCH -tip 0/8] perf-probe updates

[Ingo M. <mi...@el...>]

* Ingo Molnar <mi...@el...> wrote:

> One hickup is that -d/--del does not appear to be working yet:
> 
>   # perf probe -l
>   probe:schedule                           (on schedule)
> 
>   # perf probe -d probe:schedule
>   Fatal: Failed to write event: Invalid argument

Ah, that was with an older kernel - a freshly booted kernel with 
delete-probe syntax worked fine.

There's another small hickup i had - when i typoed 'perf probe -', it 
gave me:

 # perf probe -
 No dwarf info found in the vmlinux - please rebuild with CONFIG_DEBUG_INFO.
 An error occurred in debuginfo analysis. Try to use symbols.
   Fatal: Failed to write event: Invalid argument

Similar thing happens if i try to probe a non-existent symbol:

 # perf probe test
 No dwarf info found in the vmlinux - please rebuild with CONFIG_DEBUG_INFO.
 An error occurred in debuginfo analysis. Try to use symbols.
   Fatal: Failed to write event: Invalid argument

I think we should print something more helpful, such as:

 # perf probe test
 Fatal: Kernel symbol 'test' not found - probe not added.

the debuginfo printout is not helpful in this case - we should fall back 
to symbols silently, unless the nature of the error indicates that we 
fail _because_ there's no debuginfo.

Here the failure was because the symbol does not exist.

There's similar problems in most other failure cases. Trying to remove a 
non-existent probe gives:

 # perf probe -d test
 Warning: event "probe:test" is not found.

It should say something like:

 # perf probe -d test
 Info: event "probe:test" does not exist, could not remove it.

Also, it's possible to add multiple probes to the same function, using 
'perf probe schedule' + 'perf probe schedule', etc. While in general it 
makes sense to allow it, by default we should refuse the second, 
identical probe on the symbol - and add a -f/--force option to force 
duplicate probes. I.e. the second probe should print:

 # perf probe schedule
 Info: event "probe:schedule" already exists. (Use -f to force a duplicate.)

etc. Please try out various sensible and also less sensible options of 
this tool and try to make it break - and see whether the behavior is 
intuitive and obvious to users - whether the messages are consistent, 
etc. etc.

	Ingo

[Dle-develop] [tip:perf/urgent] perf probe: Remove event suffix number _0

[tip-bot f. M. H. <mhi...@re...>]

Commit-ID:  17f88fcd667a914b6f4dca146c9a09492fcd57b8
Gitweb:     http://git.kernel.org/tip/17f88fcd667a914b6f4dca146c9a09492fcd57b8
Author:     Masami Hiramatsu <mhi...@re...>
AuthorDate: Tue, 8 Dec 2009 17:03:02 -0500
Committer:  Ingo Molnar <mi...@el...>
CommitDate: Wed, 9 Dec 2009 07:26:52 +0100

perf probe: Remove event suffix number _0

Remove event suffix number _0 if it is the first.
The first event has no suffix, and from the second,
each event has suffix number counted from _1. This
reduces typing cost :-).

Signed-off-by: Masami Hiramatsu <mhi...@re...>
Cc: Steven Rostedt <ro...@go...>
Cc: Jim Keniston <jke...@us...>
Cc: Ananth N Mavinakayanahalli <an...@in...>
Cc: Christoph Hellwig <hc...@in...>
Cc: Frank Ch. Eigler <fc...@re...>
Cc: Frederic Weisbecker <fwe...@gm...>
Cc: Jason Baron <jb...@re...>
Cc: K.Prasad <pr...@li...>
Cc: Peter Zijlstra <pe...@in...>
Cc: Srikar Dronamraju <sr...@li...>
Cc: Arnaldo Carvalho de Melo <ac...@re...>
Cc: systemtap <sys...@so...>
Cc: DLE <dle...@li...>
LKML-Reference: <200...@dh...>
Signed-off-by: Ingo Molnar <mi...@el...>
---
 tools/perf/util/probe-event.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 2c4d301..31beedc 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -466,7 +466,16 @@ static void get_new_event_name(char *buf, size_t len, const char *base,
 			       struct strlist *namelist)
 {
 	int i, ret;
-	for (i = 0; i < MAX_EVENT_INDEX; i++) {
+
+	/* Try no suffix */
+	ret = e_snprintf(buf, len, "%s", base);
+	if (ret < 0)
+		die("snprintf() failed: %s", strerror(-ret));
+	if (!strlist__has_entry(namelist, buf))
+		return;
+
+	/* Try to add suffix */
+	for (i = 1; i < MAX_EVENT_INDEX; i++) {
 		ret = e_snprintf(buf, len, "%s_%d", base, i);
 		if (ret < 0)
 			die("snprintf() failed: %s", strerror(-ret));

[Dle-develop] [tip:perf/urgent] perf probe: Fix add-probe command syntax without --add option

[tip-bot f. M. H. <mhi...@re...>]

Commit-ID:  d1bde3f755e8652faad59e264c466c4baab68fa8
Gitweb:     http://git.kernel.org/tip/d1bde3f755e8652faad59e264c466c4baab68fa8
Author:     Masami Hiramatsu <mhi...@re...>
AuthorDate: Tue, 8 Dec 2009 17:02:54 -0500
Committer:  Ingo Molnar <mi...@el...>
CommitDate: Wed, 9 Dec 2009 07:26:51 +0100

perf probe: Fix add-probe command syntax without --add option

Fix add-probe command syntax without --add option.
perf-probe supports add-probe command without --add
option. But it treats each argument as an event definition.
e.g.

perf probe func arg1 arg2

 is interpreted as

perf probe --add func --add arg1 --add arg2

But it may be useless in many cases.

This patch fixes this syntax to fold those arguments into
one event definition if there is no --add option. With this
change, above command is interpreted as below;

perf probe --add "func arg1 arg2"

Signed-off-by: Masami Hiramatsu <mhi...@re...>
Cc: Steven Rostedt <ro...@go...>
Cc: Jim Keniston <jke...@us...>
Cc: Ananth N Mavinakayanahalli <an...@in...>
Cc: Christoph Hellwig <hc...@in...>
Cc: Frank Ch. Eigler <fc...@re...>
Cc: Frederic Weisbecker <fwe...@gm...>
Cc: Jason Baron <jb...@re...>
Cc: K.Prasad <pr...@li...>
Cc: Peter Zijlstra <pe...@in...>
Cc: Srikar Dronamraju <sr...@li...>
Cc: Arnaldo Carvalho de Melo <ac...@re...>
Cc: systemtap <sys...@so...>
Cc: DLE <dle...@li...>
LKML-Reference: <200...@dh...>
Signed-off-by: Ingo Molnar <mi...@el...>
---
 tools/perf/builtin-probe.c |   33 ++++++++++++++++++++++++++-------
 1 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 8993a1f..1347fdf 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -79,6 +79,25 @@ static void parse_probe_event(const char *str)
 	pr_debug("%d arguments\n", pp->nr_args);
 }
 
+static void parse_probe_event_argv(int argc, const char **argv)
+{
+	int i, len;
+	char *buf;
+
+	/* Bind up rest arguments */
+	len = 0;
+	for (i = 0; i < argc; i++)
+		len += strlen(argv[i]) + 1;
+	buf = zalloc(len + 1);
+	if (!buf)
+		die("Failed to allocate memory for binding arguments.");
+	len = 0;
+	for (i = 0; i < argc; i++)
+		len += sprintf(&buf[len], "%s ", argv[i]);
+	parse_probe_event(buf);
+	free(buf);
+}
+
 static int opt_add_probe_event(const struct option *opt __used,
 			      const char *str, int unset __used)
 {
@@ -160,7 +179,7 @@ static const struct option options[] = {
 
 int cmd_probe(int argc, const char **argv, const char *prefix __used)
 {
-	int i, j, ret;
+	int i, ret;
 #ifndef NO_LIBDWARF
 	int fd;
 #endif
@@ -168,8 +187,8 @@ int cmd_probe(int argc, const char **argv, const char *prefix __used)
 
 	argc = parse_options(argc, argv, options, probe_usage,
 			     PARSE_OPT_STOP_AT_NON_OPTION);
-	for (i = 0; i < argc; i++)
-		parse_probe_event(argv[i]);
+	if (argc > 0)
+		parse_probe_event_argv(argc, argv);
 
 	if ((session.nr_probe == 0 && !listing) ||
 	    (session.nr_probe != 0 && listing))
@@ -200,8 +219,8 @@ int cmd_probe(int argc, const char **argv, const char *prefix __used)
 	}
 
 	/* Searching probe points */
-	for (j = 0; j < session.nr_probe; j++) {
-		pp = &session.probes[j];
+	for (i = 0; i < session.nr_probe; i++) {
+		pp = &session.probes[i];
 		if (pp->found)
 			continue;
 
@@ -223,8 +242,8 @@ end_dwarf:
 #endif /* !NO_LIBDWARF */
 
 	/* Synthesize probes without dwarf */
-	for (j = 0; j < session.nr_probe; j++) {
-		pp = &session.probes[j];
+	for (i = 0; i < session.nr_probe; i++) {
+		pp = &session.probes[i];
 		if (pp->found)	/* This probe is already found. */
 			continue;
 

[Dle-develop] [tip:perf/urgent] perf probe: Update perf-probe document

[tip-bot f. M. H. <mhi...@re...>]

Commit-ID:  c937fe20cb6d9e24c6ad5f9f0c64d64c78411057
Gitweb:     http://git.kernel.org/tip/c937fe20cb6d9e24c6ad5f9f0c64d64c78411057
Author:     Masami Hiramatsu <mhi...@re...>
AuthorDate: Tue, 8 Dec 2009 17:03:30 -0500
Committer:  Ingo Molnar <mi...@el...>
CommitDate: Wed, 9 Dec 2009 07:26:54 +0100

perf probe: Update perf-probe document

Add --list and --del option descriptions to perf-probe.txt.

Signed-off-by: Masami Hiramatsu <mhi...@re...>
Cc: Steven Rostedt <ro...@go...>
Cc: Jim Keniston <jke...@us...>
Cc: Ananth N Mavinakayanahalli <an...@in...>
Cc: Christoph Hellwig <hc...@in...>
Cc: Frank Ch. Eigler <fc...@re...>
Cc: Frederic Weisbecker <fwe...@gm...>
Cc: Jason Baron <jb...@re...>
Cc: K.Prasad <pr...@li...>
Cc: Peter Zijlstra <pe...@in...>
Cc: Srikar Dronamraju <sr...@li...>
Cc: Arnaldo Carvalho de Melo <ac...@re...>
Cc: systemtap <sys...@so...>
Cc: DLE <dle...@li...>
Cc: Frederic Weisbecker <fwe...@gm...>
LKML-Reference: <200...@dh...>
Signed-off-by: Ingo Molnar <mi...@el...>
---
 tools/perf/Documentation/perf-probe.txt |   21 ++++++++++++++++-----
 1 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
index 9270594..8fa6bf9 100644
--- a/tools/perf/Documentation/perf-probe.txt
+++ b/tools/perf/Documentation/perf-probe.txt
@@ -8,10 +8,13 @@ perf-probe - Define new dynamic tracepoints
 SYNOPSIS
 --------
 [verse]
-'perf probe' [options] --add 'PROBE' [--add 'PROBE' ...]
+'perf probe' [options] --add='PROBE' [...]
 or
-'perf probe' [options] 'PROBE' ['PROBE' ...]
-
+'perf probe' [options] PROBE
+or
+'perf probe' [options] --del='[GROUP:]EVENT' [...]
+or
+'perf probe' --list
 
 DESCRIPTION
 -----------
@@ -31,8 +34,16 @@ OPTIONS
         Be more verbose (show parsed arguments, etc).
 
 -a::
---add::
-	Define a probe point (see PROBE SYNTAX for detail)
+--add=::
+	Define a probe event (see PROBE SYNTAX for detail).
+
+-d::
+--del=::
+	Delete a probe event.
+
+-l::
+--list::
+	List up current probe events.
 
 PROBE SYNTAX
 ------------

[Dle-develop] [tip:perf/urgent] perf probe: Change event list format

[tip-bot f. M. H. <mhi...@re...>]

Commit-ID:  278498d438781426d8f315b65f7bca023a26fcc0
Gitweb:     http://git.kernel.org/tip/278498d438781426d8f315b65f7bca023a26fcc0
Author:     Masami Hiramatsu <mhi...@re...>
AuthorDate: Tue, 8 Dec 2009 17:02:40 -0500
Committer:  Ingo Molnar <mi...@el...>
CommitDate: Wed, 9 Dec 2009 07:26:50 +0100

perf probe: Change event list format

Change event list format for user readability. perf probe --list
shows event list in "[GROUP:EVENT] EVENT-DEFINITION" format, but
this format is different from the output of perf-list, and
EVENT-DEFINITION is a bit blunt. This patch changes the format to
more user friendly one.

Before:
[probe:schedule_0]	schedule+10 prev cpu

After:
  probe:schedule_0                         (on schedule+10 with prev cpu)

Signed-off-by: Masami Hiramatsu <mhi...@re...>
Cc: Steven Rostedt <ro...@go...>
Cc: Jim Keniston <jke...@us...>
Cc: Ananth N Mavinakayanahalli <an...@in...>
Cc: Christoph Hellwig <hc...@in...>
Cc: Frank Ch. Eigler <fc...@re...>
Cc: Frederic Weisbecker <fwe...@gm...>
Cc: Jason Baron <jb...@re...>
Cc: K.Prasad <pr...@li...>
Cc: Peter Zijlstra <pe...@in...>
Cc: Srikar Dronamraju <sr...@li...>
Cc: Arnaldo Carvalho de Melo <ac...@re...>
Cc: systemtap <sys...@so...>
Cc: DLE <dle...@li...>
LKML-Reference: <200...@dh...>
Signed-off-by: Ingo Molnar <mi...@el...>
---
 tools/perf/util/probe-event.c |   27 +++++++++++++++++++++++++--
 1 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 88e1804..a20e382 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -379,11 +379,29 @@ static void clear_probe_point(struct probe_point *pp)
 	memset(pp, 0, sizeof(pp));
 }
 
+/* Show an event */
+static void show_perf_probe_event(const char *group, const char *event,
+				  const char *place, struct probe_point *pp)
+{
+	int i;
+	char buf[128];
+
+	e_snprintf(buf, 128, "%s:%s", group, event);
+	printf("  %-40s (on %s", buf, place);
+
+	if (pp->nr_args > 0) {
+		printf(" with");
+		for (i = 0; i < pp->nr_args; i++)
+			printf(" %s", pp->args[i]);
+	}
+	printf(")\n");
+}
+
 /* List up current perf-probe events */
 void show_perf_probe_events(void)
 {
 	unsigned int i;
-	int fd;
+	int fd, nr;
 	char *group, *event;
 	struct probe_point pp;
 	struct strlist *rawlist;
@@ -396,8 +414,13 @@ void show_perf_probe_events(void)
 	for (i = 0; i < strlist__nr_entries(rawlist); i++) {
 		ent = strlist__entry(rawlist, i);
 		parse_trace_kprobe_event(ent->s, &group, &event, &pp);
+		/* Synthesize only event probe point */
+		nr = pp.nr_args;
+		pp.nr_args = 0;
 		synthesize_perf_probe_event(&pp);
-		printf("[%s:%s]\t%s\n", group, event, pp.probes[0]);
+		pp.nr_args = nr;
+		/* Show an event */
+		show_perf_probe_event(group, event, pp.probes[0], &pp);
 		free(group);
 		free(event);
 		clear_probe_point(&pp);

[Dle-develop] [tip:perf/urgent] trace-kprobe: Support delete probe syntax

[tip-bot f. M. H. <mhi...@re...>]

Commit-ID:  a7c312bed772c11138409c3a98531e85d690302e
Gitweb:     http://git.kernel.org/tip/a7c312bed772c11138409c3a98531e85d690302e
Author:     Masami Hiramatsu <mhi...@re...>
AuthorDate: Tue, 8 Dec 2009 17:03:16 -0500
Committer:  Ingo Molnar <mi...@el...>
CommitDate: Wed, 9 Dec 2009 07:26:53 +0100

trace-kprobe: Support delete probe syntax

Support delete probe syntax. The syntax is "-:[group/]event".

Signed-off-by: Masami Hiramatsu <mhi...@re...>
Cc: Steven Rostedt <ro...@go...>
Cc: Jim Keniston <jke...@us...>
Cc: Ananth N Mavinakayanahalli <an...@in...>
Cc: Christoph Hellwig <hc...@in...>
Cc: Frank Ch. Eigler <fc...@re...>
Cc: Frederic Weisbecker <fwe...@gm...>
Cc: Jason Baron <jb...@re...>
Cc: K.Prasad <pr...@li...>
Cc: Peter Zijlstra <pe...@in...>
Cc: Srikar Dronamraju <sr...@li...>
Cc: Arnaldo Carvalho de Melo <ac...@re...>
Cc: systemtap <sys...@so...>
Cc: DLE <dle...@li...>
LKML-Reference: <200...@dh...>
Signed-off-by: Ingo Molnar <mi...@el...>
Cc: Steven Rostedt <ro...@go...>
Cc: Jim Keniston <jke...@us...>
Cc: Ananth N Mavinakayanahalli <an...@in...>
Cc: Christoph Hellwig <hc...@in...>
Cc: Frank Ch. Eigler <fc...@re...>
Cc: Frederic Weisbecker <fwe...@gm...>
Cc: Jason Baron <jb...@re...>
Cc: K.Prasad <pr...@li...>
Cc: Peter Zijlstra <pe...@in...>
Cc: Srikar Dronamraju <sr...@li...>
Cc: Arnaldo Carvalho de Melo <ac...@re...>
---
 kernel/trace/trace_kprobe.c |   37 ++++++++++++++++++++++++++++---------
 1 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index aff5f80..bf05fb4 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -606,23 +606,22 @@ static int create_trace_probe(int argc, char **argv)
 	 */
 	struct trace_probe *tp;
 	int i, ret = 0;
-	int is_return = 0;
+	int is_return = 0, is_delete = 0;
 	char *symbol = NULL, *event = NULL, *arg = NULL, *group = NULL;
 	unsigned long offset = 0;
 	void *addr = NULL;
 	char buf[MAX_EVENT_NAME_LEN];
 
-	if (argc < 2) {
-		pr_info("Probe point is not specified.\n");
-		return -EINVAL;
-	}
-
+	/* argc must be >= 1 */
 	if (argv[0][0] == 'p')
 		is_return = 0;
 	else if (argv[0][0] == 'r')
 		is_return = 1;
+	else if (argv[0][0] == '-')
+		is_delete = 1;
 	else {
-		pr_info("Probe definition must be started with 'p' or 'r'.\n");
+		pr_info("Probe definition must be started with 'p', 'r' or"
+			" '-'.\n");
 		return -EINVAL;
 	}
 
@@ -642,7 +641,29 @@ static int create_trace_probe(int argc, char **argv)
 			return -EINVAL;
 		}
 	}
+	if (!group)
+		group = KPROBE_EVENT_SYSTEM;
 
+	if (is_delete) {
+		if (!event) {
+			pr_info("Delete command needs an event name.\n");
+			return -EINVAL;
+		}
+		tp = find_probe_event(event, group);
+		if (!tp) {
+			pr_info("Event %s/%s doesn't exist.\n", group, event);
+			return -ENOENT;
+		}
+		/* delete an event */
+		unregister_trace_probe(tp);
+		free_trace_probe(tp);
+		return 0;
+	}
+
+	if (argc < 2) {
+		pr_info("Probe point is not specified.\n");
+		return -EINVAL;
+	}
 	if (isdigit(argv[1][0])) {
 		if (is_return) {
 			pr_info("Return probe point must be a symbol.\n");
@@ -671,8 +692,6 @@ static int create_trace_probe(int argc, char **argv)
 	argc -= 2; argv += 2;
 
 	/* setup a probe */
-	if (!group)
-		group = KPROBE_EVENT_SYSTEM;
 	if (!event) {
 		/* Make a new event name */
 		if (symbol)

[Dle-develop] [tip:perf/urgent] perf probe: Support vmlinux on cwd by default

[tip-bot f. M. H. <mhi...@re...>]

Commit-ID:  f984f03da35357b23d53e9cad29e909810857451
Gitweb:     http://git.kernel.org/tip/f984f03da35357b23d53e9cad29e909810857451
Author:     Masami Hiramatsu <mhi...@re...>
AuthorDate: Tue, 8 Dec 2009 17:03:09 -0500
Committer:  Ingo Molnar <mi...@el...>
CommitDate: Wed, 9 Dec 2009 07:26:52 +0100

perf probe: Support vmlinux on cwd by default

Support vmlinux on current working direcotry by default and
also update file-open messages.
Now perf probe searches ./vmlinux too.

Signed-off-by: Masami Hiramatsu <mhi...@re...>
Cc: Steven Rostedt <ro...@go...>
Cc: Jim Keniston <jke...@us...>
Cc: Ananth N Mavinakayanahalli <an...@in...>
Cc: Christoph Hellwig <hc...@in...>
Cc: Frank Ch. Eigler <fc...@re...>
Cc: Frederic Weisbecker <fwe...@gm...>
Cc: Jason Baron <jb...@re...>
Cc: K.Prasad <pr...@li...>
Cc: Peter Zijlstra <pe...@in...>
Cc: Srikar Dronamraju <sr...@li...>
Cc: Arnaldo Carvalho de Melo <ac...@re...>
Cc: systemtap <sys...@so...>
Cc: DLE <dle...@li...>
LKML-Reference: <200...@dh...>
Signed-off-by: Ingo Molnar <mi...@el...>
---
 tools/perf/builtin-probe.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 1347fdf..1c97e13 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -43,11 +43,12 @@
 #include "util/probe-event.h"
 
 /* Default vmlinux search paths */
-#define NR_SEARCH_PATH 3
+#define NR_SEARCH_PATH 4
 const char *default_search_path[NR_SEARCH_PATH] = {
 "/lib/modules/%s/build/vmlinux",		/* Custom build kernel */
 "/usr/lib/debug/lib/modules/%s/vmlinux",	/* Red Hat debuginfo */
 "/boot/vmlinux-debug-%s",			/* Ubuntu */
+"./vmlinux",					/* CWD */
 };
 
 #define MAX_PATH_LEN 256
@@ -205,13 +206,14 @@ int cmd_probe(int argc, const char **argv, const char *prefix __used)
 #else	/* !NO_LIBDWARF */
 		pr_debug("Some probes require debuginfo.\n");
 
-	if (session.vmlinux)
+	if (session.vmlinux) {
+		pr_debug("Try to open %s.", session.vmlinux);
 		fd = open(session.vmlinux, O_RDONLY);
-	else
+	} else
 		fd = open_default_vmlinux();
 	if (fd < 0) {
 		if (session.need_dwarf)
-			die("Could not open vmlinux/module file.");
+			die("Could not open debuginfo file.");
 
 		pr_debug("Could not open vmlinux/module file."
 			 " Try to use symbols.\n");

[Dle-develop] [tip:perf/urgent] perf probe: Support --del option

[tip-bot f. M. H. <mhi...@re...>]

Commit-ID:  fa28244d12337eebcc620b23852ec3cf29582ff9
Gitweb:     http://git.kernel.org/tip/fa28244d12337eebcc620b23852ec3cf29582ff9
Author:     Masami Hiramatsu <mhi...@re...>
AuthorDate: Tue, 8 Dec 2009 17:03:23 -0500
Committer:  Ingo Molnar <mi...@el...>
CommitDate: Wed, 9 Dec 2009 07:26:53 +0100

perf probe: Support --del option

Support perf probe --del <event> option. Currently,
perf probe can have only one event for each --del option.
If you'd like to delete several probe events, you need
to specify --del for each events.

Signed-off-by: Masami Hiramatsu <mhi...@re...>
Cc: Steven Rostedt <ro...@go...>
Cc: Jim Keniston <jke...@us...>
Cc: Ananth N Mavinakayanahalli <an...@in...>
Cc: Christoph Hellwig <hc...@in...>
Cc: Frank Ch. Eigler <fc...@re...>
Cc: Frederic Weisbecker <fwe...@gm...>
Cc: Jason Baron <jb...@re...>
Cc: K.Prasad <pr...@li...>
Cc: Peter Zijlstra <pe...@in...>
Cc: Srikar Dronamraju <sr...@li...>
Cc: Arnaldo Carvalho de Melo <ac...@re...>
Cc: systemtap <sys...@so...>
Cc: DLE <dle...@li...>
LKML-Reference: <200...@dh...>
Signed-off-by: Ingo Molnar <mi...@el...>
---
 tools/perf/builtin-probe.c    |   33 ++++++++++++++++++--
 tools/perf/util/probe-event.c |   69 ++++++++++++++++++++++++++++++++++++++--
 tools/perf/util/probe-event.h |    1 +
 3 files changed, 96 insertions(+), 7 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 1c97e13..5a47c1e 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -35,6 +35,7 @@
 #include "perf.h"
 #include "builtin.h"
 #include "util/util.h"
+#include "util/strlist.h"
 #include "util/event.h"
 #include "util/debug.h"
 #include "util/parse-options.h"
@@ -61,6 +62,7 @@ static struct {
 	int need_dwarf;
 	int nr_probe;
 	struct probe_point probes[MAX_PROBES];
+	struct strlist *dellist;
 } session;
 
 static bool listing;
@@ -107,6 +109,17 @@ static int opt_add_probe_event(const struct option *opt __used,
 	return 0;
 }
 
+static int opt_del_probe_event(const struct option *opt __used,
+			       const char *str, int unset __used)
+{
+	if (str) {
+		if (!session.dellist)
+			session.dellist = strlist__new(true, NULL);
+		strlist__add(session.dellist, str);
+	}
+	return 0;
+}
+
 #ifndef NO_LIBDWARF
 static int open_default_vmlinux(void)
 {
@@ -141,6 +154,7 @@ static int open_default_vmlinux(void)
 static const char * const probe_usage[] = {
 	"perf probe [<options>] 'PROBEDEF' ['PROBEDEF' ...]",
 	"perf probe [<options>] --add 'PROBEDEF' [--add 'PROBEDEF' ...]",
+	"perf probe [<options>] --del '[GROUP:]EVENT' ...",
 	"perf probe --list",
 	NULL
 };
@@ -152,7 +166,9 @@ static const struct option options[] = {
 	OPT_STRING('k', "vmlinux", &session.vmlinux, "file",
 		"vmlinux/module pathname"),
 #endif
-	OPT_BOOLEAN('l', "list", &listing, "list up current probes"),
+	OPT_BOOLEAN('l', "list", &listing, "list up current probe events"),
+	OPT_CALLBACK('d', "del", NULL, "[GROUP:]EVENT", "delete a probe event.",
+		opt_del_probe_event),
 	OPT_CALLBACK('a', "add", NULL,
 #ifdef NO_LIBDWARF
 		"FUNC[+OFFS|%return] [ARG ...]",
@@ -191,15 +207,26 @@ int cmd_probe(int argc, const char **argv, const char *prefix __used)
 	if (argc > 0)
 		parse_probe_event_argv(argc, argv);
 
-	if ((session.nr_probe == 0 && !listing) ||
-	    (session.nr_probe != 0 && listing))
+	if ((session.nr_probe == 0 && !session.dellist && !listing))
 		usage_with_options(probe_usage, options);
 
 	if (listing) {
+		if (session.nr_probe != 0 || session.dellist) {
+			pr_warning("  Error: Don't use --list with"
+				   " --add/--del.\n");
+			usage_with_options(probe_usage, options);
+		}
 		show_perf_probe_events();
 		return 0;
 	}
 
+	if (session.dellist) {
+		del_trace_kprobe_events(session.dellist);
+		strlist__delete(session.dellist);
+		if (session.nr_probe == 0)
+			return 0;
+	}
+
 	if (session.need_dwarf)
 #ifdef NO_LIBDWARF
 		die("Debuginfo-analysis is not supported");
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 31beedc..9480d99 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -430,10 +430,11 @@ void show_perf_probe_events(void)
 }
 
 /* Get current perf-probe event names */
-static struct strlist *get_perf_event_names(int fd)
+static struct strlist *get_perf_event_names(int fd, bool include_group)
 {
 	unsigned int i;
 	char *group, *event;
+	char buf[128];
 	struct strlist *sl, *rawlist;
 	struct str_node *ent;
 
@@ -443,7 +444,12 @@ static struct strlist *get_perf_event_names(int fd)
 	for (i = 0; i < strlist__nr_entries(rawlist); i++) {
 		ent = strlist__entry(rawlist, i);
 		parse_trace_kprobe_event(ent->s, &group, &event, NULL);
-		strlist__add(sl, event);
+		if (include_group) {
+			if (e_snprintf(buf, 128, "%s:%s", group, event) < 0)
+				die("Failed to copy group:event name.");
+			strlist__add(sl, buf);
+		} else
+			strlist__add(sl, event);
 		free(group);
 		free(event);
 	}
@@ -457,9 +463,10 @@ static void write_trace_kprobe_event(int fd, const char *buf)
 {
 	int ret;
 
+	pr_debug("Writing event: %s\n", buf);
 	ret = write(fd, buf, strlen(buf));
 	if (ret <= 0)
-		die("Failed to create event.");
+		die("Failed to write event: %s", strerror(errno));
 }
 
 static void get_new_event_name(char *buf, size_t len, const char *base,
@@ -496,7 +503,7 @@ void add_trace_kprobe_events(struct probe_point *probes, int nr_probes)
 
 	fd = open_kprobe_events(O_RDWR, O_APPEND);
 	/* Get current event names */
-	namelist = get_perf_event_names(fd);
+	namelist = get_perf_event_names(fd, false);
 
 	for (j = 0; j < nr_probes; j++) {
 		pp = probes + j;
@@ -524,3 +531,57 @@ void add_trace_kprobe_events(struct probe_point *probes, int nr_probes)
 	strlist__delete(namelist);
 	close(fd);
 }
+
+static void del_trace_kprobe_event(int fd, const char *group,
+				   const char *event, struct strlist *namelist)
+{
+	char buf[128];
+
+	if (e_snprintf(buf, 128, "%s:%s", group, event) < 0)
+		die("Failed to copy event.");
+	if (!strlist__has_entry(namelist, buf)) {
+		pr_warning("Warning: event \"%s\" is not found.\n", buf);
+		return;
+	}
+	/* Convert from perf-probe event to trace-kprobe event */
+	if (e_snprintf(buf, 128, "-:%s/%s", group, event) < 0)
+		die("Failed to copy event.");
+
+	write_trace_kprobe_event(fd, buf);
+	printf("Remove event: %s:%s\n", group, event);
+}
+
+void del_trace_kprobe_events(struct strlist *dellist)
+{
+	int fd;
+	unsigned int i;
+	const char *group, *event;
+	char *p, *str;
+	struct str_node *ent;
+	struct strlist *namelist;
+
+	fd = open_kprobe_events(O_RDWR, O_APPEND);
+	/* Get current event names */
+	namelist = get_perf_event_names(fd, true);
+
+	for (i = 0; i < strlist__nr_entries(dellist); i++) {
+		ent = strlist__entry(dellist, i);
+		str = strdup(ent->s);
+		if (!str)
+			die("Failed to copy event.");
+		p = strchr(str, ':');
+		if (p) {
+			group = str;
+			*p = '\0';
+			event = p + 1;
+		} else {
+			group = PERFPROBE_GROUP;
+			event = str;
+		}
+		del_trace_kprobe_event(fd, group, event, namelist);
+		free(str);
+	}
+	strlist__delete(namelist);
+	close(fd);
+}
+
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index 0c6fe56..f752159 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -10,6 +10,7 @@ extern void parse_trace_kprobe_event(const char *str, char **group,
 				     char **event, struct probe_point *pp);
 extern int synthesize_trace_kprobe_event(struct probe_point *pp);
 extern void add_trace_kprobe_events(struct probe_point *probes, int nr_probes);
+extern void del_trace_kprobe_events(struct strlist *dellist);
 extern void show_perf_probe_events(void);
 
 /* Maximum index number of event-name postfix */

[Dle-develop] [tip:perf/urgent] perf probe: Change probe-added message more user-friendly

[tip-bot f. M. H. <mhi...@re...>]

Commit-ID:  a9b495b0d35859971d6896293f6d0a0d880c7dfb
Gitweb:     http://git.kernel.org/tip/a9b495b0d35859971d6896293f6d0a0d880c7dfb
Author:     Masami Hiramatsu <mhi...@re...>
AuthorDate: Tue, 8 Dec 2009 17:02:47 -0500
Committer:  Ingo Molnar <mi...@el...>
CommitDate: Wed, 9 Dec 2009 07:26:51 +0100

perf probe: Change probe-added message more user-friendly

Change probe-added message more user-friendly expression and
show usage of new events.

Before:
Added new event: p:probe/schedule_0 schedule+10 prev=%ax cpu=%bx

After:
Added new event:
  probe:schedule_1                         (on schedule+1 with prev cpu)

You can now use it on all perf tools, such as:

        perf record -e probe:schedule_1 -a sleep 1

Signed-off-by: Masami Hiramatsu <mhi...@re...>
Cc: Steven Rostedt <ro...@go...>
Cc: Jim Keniston <jke...@us...>
Cc: Ananth N Mavinakayanahalli <an...@in...>
Cc: Christoph Hellwig <hc...@in...>
Cc: Frank Ch. Eigler <fc...@re...>
Cc: Frederic Weisbecker <fwe...@gm...>
Cc: Jason Baron <jb...@re...>
Cc: K.Prasad <pr...@li...>
Cc: Peter Zijlstra <pe...@in...>
Cc: Srikar Dronamraju <sr...@li...>
Cc: Arnaldo Carvalho de Melo <ac...@re...>
Cc: systemtap <sys...@so...>
Cc: DLE <dle...@li...>
LKML-Reference: <200...@dh...>
Signed-off-by: Ingo Molnar <mi...@el...>
---
 tools/perf/util/probe-event.c |   15 ++++++++++-----
 1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index a20e382..2c4d301 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -453,17 +453,13 @@ static struct strlist *get_perf_event_names(int fd)
 	return sl;
 }
 
-static int write_trace_kprobe_event(int fd, const char *buf)
+static void write_trace_kprobe_event(int fd, const char *buf)
 {
 	int ret;
 
 	ret = write(fd, buf, strlen(buf));
 	if (ret <= 0)
 		die("Failed to create event.");
-	else
-		printf("Added new event: %s\n", buf);
-
-	return ret;
 }
 
 static void get_new_event_name(char *buf, size_t len, const char *base,
@@ -503,10 +499,19 @@ void add_trace_kprobe_events(struct probe_point *probes, int nr_probes)
 				 PERFPROBE_GROUP, event,
 				 pp->probes[i]);
 			write_trace_kprobe_event(fd, buf);
+			printf("Added new event:\n");
+			/* Get the first parameter (probe-point) */
+			sscanf(pp->probes[i], "%s", buf);
+			show_perf_probe_event(PERFPROBE_GROUP, event,
+					      buf, pp);
 			/* Add added event name to namelist */
 			strlist__add(namelist, event);
 		}
 	}
+	/* Show how to use the event. */
+	printf("\nYou can now use it on all perf tools, such as:\n\n");
+	printf("\tperf record -e %s:%s -a sleep 1\n\n", PERFPROBE_GROUP, event);
+
 	strlist__delete(namelist);
 	close(fd);
 }

Re: [Dle-develop] [PATCH -tip 0/8] perf-probe updates

[Ingo M. <mi...@el...>]

* Masami Hiramatsu <mhi...@re...> wrote:

> Hi Ingo,
> 
> I made several usability updates and added --del option
> for perf-probe. I think most of basic functions are implemented.
> I'm planning to support checking build-id next, because it
> can prevent users to use old vmlinux for debuginfo analysis.
> 
> Here are the todo list I have (most of them had been requested
> on LKML). I'd like to share this list with other developers
> who are interested in.
> 
> Short-term TODOs:
>   - Support checking kernel Build-ID
> 
> Long-term TODOs (future features):
>   - Support --line option to show which lines user can probe
>   - Support lazy string matching(glob?) for selecting probing
>     line
>   - Support sys_perf_counter_open (for non-root users)
>   - Support tracing static variables (non global)
>   - Support variable types from debuginfo (e.g. char, int, ...)
>   - Support fields of data structures (var->field)
>   - Support array (var[N])
>   - Support dynamic array-indexing (var[var2])
>   - Support string/dynamic arrays (*var, var[N..M])
>   - Support force type-casting ((type)var)
>   - Support the type of return value
> 
> Miscs:
>   - Support glob expression with --del option (like --del "*")
>   - Support event/group name specifying for new events
>   - Better support for probes on modules
>   - Symbol search by libelf/kallsyms
>   - Move onto libdw/libdwfl
>   - Storing file name/line number information in the
>     kernel for listing events
>   
> 
> Thank you,
> 
> ---
> 
> Masami Hiramatsu (8):
>       perf probe: Update perf-probe document
>       perf probe: Support --del option
>       trace-kprobe: Support delete probe syntax
>       perf probe: Support vmlinux on cwd by default
>       perf probe: Remove event suffix number _0
>       perf probe: Fix add-probe command syntax without --add option
>       perf probe: Change probe-added message more user-friendly
>       perf probe: Change event list format
> 
> 
>  kernel/trace/trace_kprobe.c             |   37 +++++++--
>  tools/perf/Documentation/perf-probe.txt |   21 ++++-
>  tools/perf/builtin-probe.c              |   76 ++++++++++++++++---
>  tools/perf/util/probe-event.c           |  122 ++++++++++++++++++++++++++++---
>  tools/perf/util/probe-event.h           |    1 
>  5 files changed, 217 insertions(+), 40 deletions(-)

Applied - thanks Masami!

These are very nice enhancements. I tried out the new features and they 
worked well, and the use of the tool is intuitive.

One hickup is that -d/--del does not appear to be working yet:

  # perf probe -l
  probe:schedule                           (on schedule)

  # perf probe -d probe:schedule
  Fatal: Failed to write event: Invalid argument

	Ingo

Re: [Dle-develop] [PATCH v3] [RFC] tracepoint: Add signal coredump tracepoint

[KOSAKI M. <kos...@jp...>]

> +	TP_fast_assign(
> +		__entry->sig	= (int)cprm->signr;
> +		__entry->limit	= cprm->limit;
> +		__entry->flags	= cprm->mm_flags;
> +		__entry->retval	= retval;
> +		__assign_str(name,	core_name);
> +	),
> +
> +	TP_printk("sig=%d limit=%lu dumpable=0x%lx dump_filter=0x%lx "
> +		  "corename=\"%s\" retval=%d",
> +		  __entry->sig, __entry->limit,
> +		  __entry->flags & MMF_DUMPABLE_MASK,
> +		  (__entry->flags & MMF_DUMP_FILTER_MASK) >>
> +		  MMF_DUMP_FILTER_SHIFT,
> +		  __get_str(name), __entry->retval)
> +);
>  #endif /* _TRACE_SIGNAL_H */

I don't think "limit" is userfriendly name, core_limit or core_size_limit is better?
plus, we have core_pipe_limit sysctl too. (it's similar but different concept limit).

other parts looks good to me.

[Dle-develop] [PATCH -tip 8/8] perf probe: Update perf-probe document

[Masami H. <mhi...@re...>]

Add --list and --del option descriptions to perf-probe.txt.

Signed-off-by: Masami Hiramatsu <mhi...@re...>
Cc: Ingo Molnar <mi...@el...>
Cc: Steven Rostedt <ro...@go...>
Cc: Jim Keniston <jke...@us...>
Cc: Ananth N Mavinakayanahalli <an...@in...>
Cc: Christoph Hellwig <hc...@in...>
Cc: Frank Ch. Eigler <fc...@re...>
Cc: Frederic Weisbecker <fwe...@gm...>
Cc: Jason Baron <jb...@re...>
Cc: K.Prasad <pr...@li...>
Cc: Peter Zijlstra <pe...@in...>
Cc: Srikar Dronamraju <sr...@li...>
Cc: Arnaldo Carvalho de Melo <ac...@re...>
---

 tools/perf/Documentation/perf-probe.txt |   21 ++++++++++++++++-----
 1 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
index 9270594..8fa6bf9 100644
--- a/tools/perf/Documentation/perf-probe.txt
+++ b/tools/perf/Documentation/perf-probe.txt
@@ -8,10 +8,13 @@ perf-probe - Define new dynamic tracepoints
 SYNOPSIS
 --------
 [verse]
-'perf probe' [options] --add 'PROBE' [--add 'PROBE' ...]
+'perf probe' [options] --add='PROBE' [...]
 or
-'perf probe' [options] 'PROBE' ['PROBE' ...]
-
+'perf probe' [options] PROBE
+or
+'perf probe' [options] --del='[GROUP:]EVENT' [...]
+or
+'perf probe' --list
 
 DESCRIPTION
 -----------
@@ -31,8 +34,16 @@ OPTIONS
         Be more verbose (show parsed arguments, etc).
 
 -a::
---add::
-	Define a probe point (see PROBE SYNTAX for detail)
+--add=::
+	Define a probe event (see PROBE SYNTAX for detail).
+
+-d::
+--del=::
+	Delete a probe event.
+
+-l::
+--list::
+	List up current probe events.
 
 PROBE SYNTAX
 ------------


-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhi...@re...

[Dle-develop] [PATCH -tip 7/8] perf probe: Support --del option

[Masami H. <mhi...@re...>]

Support perf probe --del <event> option. Currently,
perf probe can have only one event for each --del option.
If you'd like to delete several probe events, you need
to specify --del for each events.

Signed-off-by: Masami Hiramatsu <mhi...@re...>
Cc: Ingo Molnar <mi...@el...>
Cc: Steven Rostedt <ro...@go...>
Cc: Jim Keniston <jke...@us...>
Cc: Ananth N Mavinakayanahalli <an...@in...>
Cc: Christoph Hellwig <hc...@in...>
Cc: Frank Ch. Eigler <fc...@re...>
Cc: Frederic Weisbecker <fwe...@gm...>
Cc: Jason Baron <jb...@re...>
Cc: K.Prasad <pr...@li...>
Cc: Peter Zijlstra <pe...@in...>
Cc: Srikar Dronamraju <sr...@li...>
Cc: Arnaldo Carvalho de Melo <ac...@re...>
---

 tools/perf/builtin-probe.c    |   33 ++++++++++++++++++--
 tools/perf/util/probe-event.c |   69 +++++++++++++++++++++++++++++++++++++++--
 tools/perf/util/probe-event.h |    1 +
 3 files changed, 96 insertions(+), 7 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 1c97e13..5a47c1e 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -35,6 +35,7 @@
 #include "perf.h"
 #include "builtin.h"
 #include "util/util.h"
+#include "util/strlist.h"
 #include "util/event.h"
 #include "util/debug.h"
 #include "util/parse-options.h"
@@ -61,6 +62,7 @@ static struct {
 	int need_dwarf;
 	int nr_probe;
 	struct probe_point probes[MAX_PROBES];
+	struct strlist *dellist;
 } session;
 
 static bool listing;
@@ -107,6 +109,17 @@ static int opt_add_probe_event(const struct option *opt __used,
 	return 0;
 }
 
+static int opt_del_probe_event(const struct option *opt __used,
+			       const char *str, int unset __used)
+{
+	if (str) {
+		if (!session.dellist)
+			session.dellist = strlist__new(true, NULL);
+		strlist__add(session.dellist, str);
+	}
+	return 0;
+}
+
 #ifndef NO_LIBDWARF
 static int open_default_vmlinux(void)
 {
@@ -141,6 +154,7 @@ static int open_default_vmlinux(void)
 static const char * const probe_usage[] = {
 	"perf probe [<options>] 'PROBEDEF' ['PROBEDEF' ...]",
 	"perf probe [<options>] --add 'PROBEDEF' [--add 'PROBEDEF' ...]",
+	"perf probe [<options>] --del '[GROUP:]EVENT' ...",
 	"perf probe --list",
 	NULL
 };
@@ -152,7 +166,9 @@ static const struct option options[] = {
 	OPT_STRING('k', "vmlinux", &session.vmlinux, "file",
 		"vmlinux/module pathname"),
 #endif
-	OPT_BOOLEAN('l', "list", &listing, "list up current probes"),
+	OPT_BOOLEAN('l', "list", &listing, "list up current probe events"),
+	OPT_CALLBACK('d', "del", NULL, "[GROUP:]EVENT", "delete a probe event.",
+		opt_del_probe_event),
 	OPT_CALLBACK('a', "add", NULL,
 #ifdef NO_LIBDWARF
 		"FUNC[+OFFS|%return] [ARG ...]",
@@ -191,15 +207,26 @@ int cmd_probe(int argc, const char **argv, const char *prefix __used)
 	if (argc > 0)
 		parse_probe_event_argv(argc, argv);
 
-	if ((session.nr_probe == 0 && !listing) ||
-	    (session.nr_probe != 0 && listing))
+	if ((session.nr_probe == 0 && !session.dellist && !listing))
 		usage_with_options(probe_usage, options);
 
 	if (listing) {
+		if (session.nr_probe != 0 || session.dellist) {
+			pr_warning("  Error: Don't use --list with"
+				   " --add/--del.\n");
+			usage_with_options(probe_usage, options);
+		}
 		show_perf_probe_events();
 		return 0;
 	}
 
+	if (session.dellist) {
+		del_trace_kprobe_events(session.dellist);
+		strlist__delete(session.dellist);
+		if (session.nr_probe == 0)
+			return 0;
+	}
+
 	if (session.need_dwarf)
 #ifdef NO_LIBDWARF
 		die("Debuginfo-analysis is not supported");
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 31beedc..9480d99 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -430,10 +430,11 @@ void show_perf_probe_events(void)
 }
 
 /* Get current perf-probe event names */
-static struct strlist *get_perf_event_names(int fd)
+static struct strlist *get_perf_event_names(int fd, bool include_group)
 {
 	unsigned int i;
 	char *group, *event;
+	char buf[128];
 	struct strlist *sl, *rawlist;
 	struct str_node *ent;
 
@@ -443,7 +444,12 @@ static struct strlist *get_perf_event_names(int fd)
 	for (i = 0; i < strlist__nr_entries(rawlist); i++) {
 		ent = strlist__entry(rawlist, i);
 		parse_trace_kprobe_event(ent->s, &group, &event, NULL);
-		strlist__add(sl, event);
+		if (include_group) {
+			if (e_snprintf(buf, 128, "%s:%s", group, event) < 0)
+				die("Failed to copy group:event name.");
+			strlist__add(sl, buf);
+		} else
+			strlist__add(sl, event);
 		free(group);
 		free(event);
 	}
@@ -457,9 +463,10 @@ static void write_trace_kprobe_event(int fd, const char *buf)
 {
 	int ret;
 
+	pr_debug("Writing event: %s\n", buf);
 	ret = write(fd, buf, strlen(buf));
 	if (ret <= 0)
-		die("Failed to create event.");
+		die("Failed to write event: %s", strerror(errno));
 }
 
 static void get_new_event_name(char *buf, size_t len, const char *base,
@@ -496,7 +503,7 @@ void add_trace_kprobe_events(struct probe_point *probes, int nr_probes)
 
 	fd = open_kprobe_events(O_RDWR, O_APPEND);
 	/* Get current event names */
-	namelist = get_perf_event_names(fd);
+	namelist = get_perf_event_names(fd, false);
 
 	for (j = 0; j < nr_probes; j++) {
 		pp = probes + j;
@@ -524,3 +531,57 @@ void add_trace_kprobe_events(struct probe_point *probes, int nr_probes)
 	strlist__delete(namelist);
 	close(fd);
 }
+
+static void del_trace_kprobe_event(int fd, const char *group,
+				   const char *event, struct strlist *namelist)
+{
+	char buf[128];
+
+	if (e_snprintf(buf, 128, "%s:%s", group, event) < 0)
+		die("Failed to copy event.");
+	if (!strlist__has_entry(namelist, buf)) {
+		pr_warning("Warning: event \"%s\" is not found.\n", buf);
+		return;
+	}
+	/* Convert from perf-probe event to trace-kprobe event */
+	if (e_snprintf(buf, 128, "-:%s/%s", group, event) < 0)
+		die("Failed to copy event.");
+
+	write_trace_kprobe_event(fd, buf);
+	printf("Remove event: %s:%s\n", group, event);
+}
+
+void del_trace_kprobe_events(struct strlist *dellist)
+{
+	int fd;
+	unsigned int i;
+	const char *group, *event;
+	char *p, *str;
+	struct str_node *ent;
+	struct strlist *namelist;
+
+	fd = open_kprobe_events(O_RDWR, O_APPEND);
+	/* Get current event names */
+	namelist = get_perf_event_names(fd, true);
+
+	for (i = 0; i < strlist__nr_entries(dellist); i++) {
+		ent = strlist__entry(dellist, i);
+		str = strdup(ent->s);
+		if (!str)
+			die("Failed to copy event.");
+		p = strchr(str, ':');
+		if (p) {
+			group = str;
+			*p = '\0';
+			event = p + 1;
+		} else {
+			group = PERFPROBE_GROUP;
+			event = str;
+		}
+		del_trace_kprobe_event(fd, group, event, namelist);
+		free(str);
+	}
+	strlist__delete(namelist);
+	close(fd);
+}
+
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index 0c6fe56..f752159 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -10,6 +10,7 @@ extern void parse_trace_kprobe_event(const char *str, char **group,
 				     char **event, struct probe_point *pp);
 extern int synthesize_trace_kprobe_event(struct probe_point *pp);
 extern void add_trace_kprobe_events(struct probe_point *probes, int nr_probes);
+extern void del_trace_kprobe_events(struct strlist *dellist);
 extern void show_perf_probe_events(void);
 
 /* Maximum index number of event-name postfix */


-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhi...@re...

[Dle-develop] [PATCH -tip 6/8] trace-kprobe: Support delete probe syntax

[Masami H. <mhi...@re...>]

Support delete probe syntax. The syntax is "-:[group/]event".

Signed-off-by: Masami Hiramatsu <mhi...@re...>
Cc: Ingo Molnar <mi...@el...>
Cc: Steven Rostedt <ro...@go...>
Cc: Jim Keniston <jke...@us...>
Cc: Ananth N Mavinakayanahalli <an...@in...>
Cc: Christoph Hellwig <hc...@in...>
Cc: Frank Ch. Eigler <fc...@re...>
Cc: Frederic Weisbecker <fwe...@gm...>
Cc: Jason Baron <jb...@re...>
Cc: K.Prasad <pr...@li...>
Cc: Peter Zijlstra <pe...@in...>
Cc: Srikar Dronamraju <sr...@li...>
Cc: Arnaldo Carvalho de Melo <ac...@re...>
---

 kernel/trace/trace_kprobe.c |   37 ++++++++++++++++++++++++++++---------
 1 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index aff5f80..bf05fb4 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -606,23 +606,22 @@ static int create_trace_probe(int argc, char **argv)
 	 */
 	struct trace_probe *tp;
 	int i, ret = 0;
-	int is_return = 0;
+	int is_return = 0, is_delete = 0;
 	char *symbol = NULL, *event = NULL, *arg = NULL, *group = NULL;
 	unsigned long offset = 0;
 	void *addr = NULL;
 	char buf[MAX_EVENT_NAME_LEN];
 
-	if (argc < 2) {
-		pr_info("Probe point is not specified.\n");
-		return -EINVAL;
-	}
-
+	/* argc must be >= 1 */
 	if (argv[0][0] == 'p')
 		is_return = 0;
 	else if (argv[0][0] == 'r')
 		is_return = 1;
+	else if (argv[0][0] == '-')
+		is_delete = 1;
 	else {
-		pr_info("Probe definition must be started with 'p' or 'r'.\n");
+		pr_info("Probe definition must be started with 'p', 'r' or"
+			" '-'.\n");
 		return -EINVAL;
 	}
 
@@ -642,7 +641,29 @@ static int create_trace_probe(int argc, char **argv)
 			return -EINVAL;
 		}
 	}
+	if (!group)
+		group = KPROBE_EVENT_SYSTEM;
 
+	if (is_delete) {
+		if (!event) {
+			pr_info("Delete command needs an event name.\n");
+			return -EINVAL;
+		}
+		tp = find_probe_event(event, group);
+		if (!tp) {
+			pr_info("Event %s/%s doesn't exist.\n", group, event);
+			return -ENOENT;
+		}
+		/* delete an event */
+		unregister_trace_probe(tp);
+		free_trace_probe(tp);
+		return 0;
+	}
+
+	if (argc < 2) {
+		pr_info("Probe point is not specified.\n");
+		return -EINVAL;
+	}
 	if (isdigit(argv[1][0])) {
 		if (is_return) {
 			pr_info("Return probe point must be a symbol.\n");
@@ -671,8 +692,6 @@ static int create_trace_probe(int argc, char **argv)
 	argc -= 2; argv += 2;
 
 	/* setup a probe */
-	if (!group)
-		group = KPROBE_EVENT_SYSTEM;
 	if (!event) {
 		/* Make a new event name */
 		if (symbol)


-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhi...@re...

[Dle-develop] [PATCH -tip 5/8] perf probe: Support vmlinux on cwd by default

[Masami H. <mhi...@re...>]

Support vmlinux on current working direcotry by default and
also update file-open messages.
Now perf probe searches ./vmlinux too.

Signed-off-by: Masami Hiramatsu <mhi...@re...>
Cc: Ingo Molnar <mi...@el...>
Cc: Steven Rostedt <ro...@go...>
Cc: Jim Keniston <jke...@us...>
Cc: Ananth N Mavinakayanahalli <an...@in...>
Cc: Christoph Hellwig <hc...@in...>
Cc: Frank Ch. Eigler <fc...@re...>
Cc: Frederic Weisbecker <fwe...@gm...>
Cc: Jason Baron <jb...@re...>
Cc: K.Prasad <pr...@li...>
Cc: Peter Zijlstra <pe...@in...>
Cc: Srikar Dronamraju <sr...@li...>
Cc: Arnaldo Carvalho de Melo <ac...@re...>
---

 tools/perf/builtin-probe.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 1347fdf..1c97e13 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -43,11 +43,12 @@
 #include "util/probe-event.h"
 
 /* Default vmlinux search paths */
-#define NR_SEARCH_PATH 3
+#define NR_SEARCH_PATH 4
 const char *default_search_path[NR_SEARCH_PATH] = {
 "/lib/modules/%s/build/vmlinux",		/* Custom build kernel */
 "/usr/lib/debug/lib/modules/%s/vmlinux",	/* Red Hat debuginfo */
 "/boot/vmlinux-debug-%s",			/* Ubuntu */
+"./vmlinux",					/* CWD */
 };
 
 #define MAX_PATH_LEN 256
@@ -205,13 +206,14 @@ int cmd_probe(int argc, const char **argv, const char *prefix __used)
 #else	/* !NO_LIBDWARF */
 		pr_debug("Some probes require debuginfo.\n");
 
-	if (session.vmlinux)
+	if (session.vmlinux) {
+		pr_debug("Try to open %s.", session.vmlinux);
 		fd = open(session.vmlinux, O_RDONLY);
-	else
+	} else
 		fd = open_default_vmlinux();
 	if (fd < 0) {
 		if (session.need_dwarf)
-			die("Could not open vmlinux/module file.");
+			die("Could not open debuginfo file.");
 
 		pr_debug("Could not open vmlinux/module file."
 			 " Try to use symbols.\n");


-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhi...@re...

[Dle-develop] [PATCH -tip 3/8] perf probe: Fix add-probe command syntax without --add option

[Masami H. <mhi...@re...>]

Fix add-probe command syntax without --add option.
perf-probe supports add-probe command without --add
option. But it treats each argument as an event definition.
e.g.

perf probe func arg1 arg2

 is interpreted as

perf probe --add func --add arg1 --add arg2

But it may be useless in many cases.

This patch fixes this syntax to fold those arguments into
one event definition if there is no --add option. With this
change, above command is interpreted as below;

perf probe --add "func arg1 arg2"

Signed-off-by: Masami Hiramatsu <mhi...@re...>
Cc: Ingo Molnar <mi...@el...>
Cc: Steven Rostedt <ro...@go...>
Cc: Jim Keniston <jke...@us...>
Cc: Ananth N Mavinakayanahalli <an...@in...>
Cc: Christoph Hellwig <hc...@in...>
Cc: Frank Ch. Eigler <fc...@re...>
Cc: Frederic Weisbecker <fwe...@gm...>
Cc: Jason Baron <jb...@re...>
Cc: K.Prasad <pr...@li...>
Cc: Peter Zijlstra <pe...@in...>
Cc: Srikar Dronamraju <sr...@li...>
Cc: Arnaldo Carvalho de Melo <ac...@re...>
---

 tools/perf/builtin-probe.c |   33 ++++++++++++++++++++++++++-------
 1 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 8993a1f..1347fdf 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -79,6 +79,25 @@ static void parse_probe_event(const char *str)
 	pr_debug("%d arguments\n", pp->nr_args);
 }
 
+static void parse_probe_event_argv(int argc, const char **argv)
+{
+	int i, len;
+	char *buf;
+
+	/* Bind up rest arguments */
+	len = 0;
+	for (i = 0; i < argc; i++)
+		len += strlen(argv[i]) + 1;
+	buf = zalloc(len + 1);
+	if (!buf)
+		die("Failed to allocate memory for binding arguments.");
+	len = 0;
+	for (i = 0; i < argc; i++)
+		len += sprintf(&buf[len], "%s ", argv[i]);
+	parse_probe_event(buf);
+	free(buf);
+}
+
 static int opt_add_probe_event(const struct option *opt __used,
 			      const char *str, int unset __used)
 {
@@ -160,7 +179,7 @@ static const struct option options[] = {
 
 int cmd_probe(int argc, const char **argv, const char *prefix __used)
 {
-	int i, j, ret;
+	int i, ret;
 #ifndef NO_LIBDWARF
 	int fd;
 #endif
@@ -168,8 +187,8 @@ int cmd_probe(int argc, const char **argv, const char *prefix __used)
 
 	argc = parse_options(argc, argv, options, probe_usage,
 			     PARSE_OPT_STOP_AT_NON_OPTION);
-	for (i = 0; i < argc; i++)
-		parse_probe_event(argv[i]);
+	if (argc > 0)
+		parse_probe_event_argv(argc, argv);
 
 	if ((session.nr_probe == 0 && !listing) ||
 	    (session.nr_probe != 0 && listing))
@@ -200,8 +219,8 @@ int cmd_probe(int argc, const char **argv, const char *prefix __used)
 	}
 
 	/* Searching probe points */
-	for (j = 0; j < session.nr_probe; j++) {
-		pp = &session.probes[j];
+	for (i = 0; i < session.nr_probe; i++) {
+		pp = &session.probes[i];
 		if (pp->found)
 			continue;
 
@@ -223,8 +242,8 @@ end_dwarf:
 #endif /* !NO_LIBDWARF */
 
 	/* Synthesize probes without dwarf */
-	for (j = 0; j < session.nr_probe; j++) {
-		pp = &session.probes[j];
+	for (i = 0; i < session.nr_probe; i++) {
+		pp = &session.probes[i];
 		if (pp->found)	/* This probe is already found. */
 			continue;
 


-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhi...@re...