* multip register_uprobe @ 2007-07-06 8:03 Wenji Huang 2007-07-06 12:35 ` Srikar Dronamraju 0 siblings, 1 reply; 5+ messages in thread From: Wenji Huang @ 2007-07-06 8:03 UTC (permalink / raw) To: systemtap Hi, I am creating some test scripts based on sarikar's framework. There is one problem about register multiple times. ................. probes->vaddr = vaddr; probes->pid = pid; probes->handler = handler; for (i=0; i < 3; i++) { ret = register_uprobe(probes); if (ret!=0) //of course, the first time should succeed and will failed in second time, ret = -16 { I tried several possibilities: * return ret; //RESULT: the module can't be removed, the probed process will hang, can't be killed/continued * return 0; //RESULT: same as above * unregister_uprobe(probes);kfree(probes);break; //RESULT: same as above * kfree(probes);break; //RESULT: the module can be removed, the probed process will hang, can't be killed/continued } } return 0; ................. But, no problem in "register once and unregister multiple times". Environment: kernel: 2.6.21 utrace: http://people.redhat.com/roland/utrace/2.6.21/ uprobe: mail by Jim, on 2007/05/26 All the tests in srikar's testsuite passed in the box. Any analysis? or need to update utrace/uprobe? Is there any website or cvs to get the resource? Regards, -- ***Wenji Huang * ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: multip register_uprobe 2007-07-06 8:03 multip register_uprobe Wenji Huang @ 2007-07-06 12:35 ` Srikar Dronamraju 2007-07-06 17:37 ` Jim Keniston 0 siblings, 1 reply; 5+ messages in thread From: Srikar Dronamraju @ 2007-07-06 12:35 UTC (permalink / raw) To: Wenji Huang; +Cc: systemtap Hi Wenji, > I am creating some test scripts based on sarikar's framework. > > There is one problem about register multiple times. > ................. > probes->vaddr = vaddr; > probes->pid = pid; > probes->handler = handler; > for (i=0; i < 3; i++) { > ret = register_uprobe(probes); > if (ret!=0) //of course, the first time should succeed and > will failed in second time, ret = -16 > { > I tried several possibilities: > * return ret; //RESULT: the module can't be removed, > the probed process will hang, can't be killed/continued > * return 0; //RESULT: same as above > * unregister_uprobe(probes);kfree(probes);break; > //RESULT: same as above > * kfree(probes);break; //RESULT: the module can be > removed, the probed process will hang, can't be killed/continued > } > } > return 0; uprobes doesn't allow to register using the same probe structure. Thats currently documented in the Documentation/uprobes.txt Under Uprobes features and Limitations. Uprobes may produce unexpected results if you: - assign non-zero values to reserved members of struct uprobe; - change the contents of a uprobe or uretprobe object while it is registered; or - attempt to register a uprobe or uretprobe that is already registered. Please do let me know if I it doesn't answer or if I have misunderstood your query. -- Thanks and Regards Srikar ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: multip register_uprobe 2007-07-06 12:35 ` Srikar Dronamraju @ 2007-07-06 17:37 ` Jim Keniston 2007-07-06 18:50 ` Jim Keniston 0 siblings, 1 reply; 5+ messages in thread From: Jim Keniston @ 2007-07-06 17:37 UTC (permalink / raw) To: Srikar Dronamraju; +Cc: Wenji Huang, systemtap On Fri, 2007-07-06 at 18:05 +0530, Srikar Dronamraju wrote: > Hi Wenji, > > > > I am creating some test scripts based on sarikar's framework. > > > > There is one problem about register multiple times. > > ................. > > probes->vaddr = vaddr; > > probes->pid = pid; > > probes->handler = handler; > > for (i=0; i < 3; i++) { > > ret = register_uprobe(probes); > > if (ret!=0) //of course, the first time should succeed and > > will failed in second time, ret = -16 > > { > > I tried several possibilities: > > * return ret; //RESULT: the module can't be removed, > > the probed process will hang, can't be killed/continued The above approach is incorrect because you fail the insmod but leave the probe registered. (I know, you're trying lots of different things...) > > * return 0; //RESULT: same as above The above approach should work, assuming your cleanup function unregisters the probe. You have found a bug. Thanks! I will post a fix shortly. > > * unregister_uprobe(probes);kfree(probes);break; > > //RESULT: same as above The above approach should work, so long as your cleanup function "knows" that the probes object has been kfreed, and doesn't try to unregister it. > > * kfree(probes);break; //RESULT: the module can be > > removed, the probed process will hang, can't be killed/continued The above approach is incorrect because you kfree a uprobe that's still registered. > > } > > } > > return 0; > > But, no problem in "register once and unregister multiple times". Yeah, unregistering an already-unregistered probe is treated as a no-op. This allows your cleanup function to unregister its probes even if the probed process has exited (at which time uprobes unregisters them on its own). > > > uprobes doesn't allow to register using the same probe structure. > Thats currently documented in the Documentation/uprobes.txt > Under Uprobes features and Limitations. > > Uprobes may produce unexpected results if you: > - assign non-zero values to reserved members of struct uprobe; > - change the contents of a uprobe or uretprobe object while it is > registered; or > - attempt to register a uprobe or uretprobe that is already registered. Correct. But uprobes should handle such a failure more gracefully. It tries to, but in this failure mode, it neglects to unlock uproc->rwsem. > > Please do let me know if I it doesn't answer or if I have misunderstood > your query. > > -- > Thanks and Regards > Srikar Thanks. Jim ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: multip register_uprobe 2007-07-06 17:37 ` Jim Keniston @ 2007-07-06 18:50 ` Jim Keniston 2007-07-09 8:19 ` Wenji Huang 0 siblings, 1 reply; 5+ messages in thread From: Jim Keniston @ 2007-07-06 18:50 UTC (permalink / raw) To: Wenji Huang; +Cc: systemtap [-- Attachment #1: Type: text/plain, Size: 1083 bytes --] On Fri, 2007-07-06 at 09:37 -0700, Jim Keniston wrote: > On Fri, 2007-07-06 at 18:05 +0530, Srikar Dronamraju wrote: > > Hi Wenji, > > > > > > > I am creating some test scripts based on sarikar's framework. > > > > > > There is one problem about register multiple times. > > > ................. > > > probes->vaddr = vaddr; > > > probes->pid = pid; > > > probes->handler = handler; > > > for (i=0; i < 3; i++) { > > > ret = register_uprobe(probes); > > > if (ret!=0) //of course, the first time should succeed and > > > will failed in second time, ret = -16 > > > { > > > I tried several possibilities: ... > > > > * return 0; //RESULT: same as above > The above approach should work, assuming your cleanup function > unregisters the probe. > > You have found a bug. Thanks! I will post a fix shortly. > Attached are a patch with the fix and a module (wenji3x.c) that demonstrates the bug (or fix). This patch applies atop the May 26 (GMT) uprobes patch set. Thanks again. Jim [-- Attachment #2: reused-probe-fix.patch --] [-- Type: text/x-patch, Size: 818 bytes --] If register_uprobe() or register_uretprobe() fails while attempting to register a probe on a process that already has one or more u[ret]probes in place, subsequent uprobes activity on that process (probe hits, register/unregister attempts) may hang. Here's a fix. --- kernel/uprobes.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff -puN kernel/uprobes.c~reused-probe-fix kernel/uprobes.c --- linux-2.6.21-rc6/kernel/uprobes.c~reused-probe-fix 2007-07-06 09:16:49.000000000 -0700 +++ linux-2.6.21-rc6-jimk/kernel/uprobes.c 2007-07-06 09:18:56.000000000 -0700 @@ -830,8 +830,10 @@ fail_uproc: if (uproc_is_new) { uprobe_free_process(uproc); mutex_unlock(&uproc_mutex); - } else + } else { + up_write(&uproc->rwsem); uprobe_put_process(uproc); + } fail_tsk: put_task_struct(p); _ [-- Attachment #3: wenji3x.c --] [-- Type: text/x-csrc, Size: 1659 bytes --] /* uprobe_example.c, modified to try to register the same probe 3 times */ #include <linux/module.h> #include <linux/kernel.h> #include <linux/init.h> #include <linux/uprobes.h> /* * Usage: insmod wenji3x.ko pid=<pid> vaddr=<address> [verbose=0] * where <pid> identifies the probed process and <address> is the virtual * address of the probed instruction. * * insmod should report success (0) for the first registration attempt * and failure (-16 = EBUSY) for the second and third attempts. The * probe should work. */ static int pid = 0; module_param(pid, int, 0); MODULE_PARM_DESC(pid, "pid"); static int verbose = 1; module_param(verbose, int, 0); MODULE_PARM_DESC(verbose, "verbose"); static long vaddr = 0; module_param(vaddr, long, 0); MODULE_PARM_DESC(vaddr, "vaddr"); static int nhits; static struct uprobe usp; static void uprobe_handler(struct uprobe *u, struct pt_regs *regs) { nhits++; if (verbose) printk(KERN_INFO "Hit #%d on probepoint at %#lx\n", nhits, u->vaddr); } int __init init_module(void) { int ret, try, success = 0; usp.pid = pid; usp.vaddr = vaddr; usp.handler = uprobe_handler; printk(KERN_INFO "Registering uprobe on pid %d, vaddr %#lx\n", usp.pid, usp.vaddr); for (try = 1; try <= 3; try++) { ret = register_uprobe(&usp); printk(KERN_ERR "Try #%d: register_uprobe() returned %d\n", try, ret); if (ret == 0) success++; } return (success ? 0 : ret); } void __exit cleanup_module(void) { printk(KERN_INFO "Unregistering uprobe on pid %d, vaddr %#lx\n", usp.pid, usp.vaddr); printk(KERN_INFO "Probepoint was hit %d times\n", nhits); unregister_uprobe(&usp); } MODULE_LICENSE("GPL"); ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: multip register_uprobe 2007-07-06 18:50 ` Jim Keniston @ 2007-07-09 8:19 ` Wenji Huang 0 siblings, 0 replies; 5+ messages in thread From: Wenji Huang @ 2007-07-09 8:19 UTC (permalink / raw) To: systemtap Hi, Jim Got it, I also tried the several cases by changing hander/vaddr, but using same probe. Also works, no problem. Thanks, Wenji Jim Keniston wrote: >On Fri, 2007-07-06 at 09:37 -0700, Jim Keniston wrote: > > >>On Fri, 2007-07-06 at 18:05 +0530, Srikar Dronamraju wrote: >> >> >>>Hi Wenji, >>> >>> >>> >>> >>>> I am creating some test scripts based on sarikar's framework. >>>> >>>> There is one problem about register multiple times. >>>> ................. >>>> probes->vaddr = vaddr; >>>> probes->pid = pid; >>>> probes->handler = handler; >>>> for (i=0; i < 3; i++) { >>>> ret = register_uprobe(probes); >>>> if (ret!=0) //of course, the first time should succeed and >>>>will failed in second time, ret = -16 >>>> { >>>>I tried several possibilities: >>>> >>>> >... > > >>>> * return 0; //RESULT: same as above >>>> >>>> >>The above approach should work, assuming your cleanup function >>unregisters the probe. >> >>You have found a bug. Thanks! I will post a fix shortly. >> >> >> > >Attached are a patch with the fix and a module (wenji3x.c) that >demonstrates the bug (or fix). This patch applies atop the May 26 (GMT) >uprobes patch set. > >Thanks again. >Jim > > >------------------------------------------------------------------------ > > >If register_uprobe() or register_uretprobe() fails while attempting to >register a probe on a process that already has one or more u[ret]probes >in place, subsequent uprobes activity on that process (probe hits, >register/unregister attempts) may hang. Here's a fix. > >--- > > kernel/uprobes.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > >diff -puN kernel/uprobes.c~reused-probe-fix kernel/uprobes.c >--- linux-2.6.21-rc6/kernel/uprobes.c~reused-probe-fix 2007-07-06 09:16:49.000000000 -0700 >+++ linux-2.6.21-rc6-jimk/kernel/uprobes.c 2007-07-06 09:18:56.000000000 -0700 >@@ -830,8 +830,10 @@ fail_uproc: > if (uproc_is_new) { > uprobe_free_process(uproc); > mutex_unlock(&uproc_mutex); >- } else >+ } else { >+ up_write(&uproc->rwsem); > uprobe_put_process(uproc); >+ } > > fail_tsk: > put_task_struct(p); >_ > > >------------------------------------------------------------------------ > >/* uprobe_example.c, modified to try to register the same probe 3 times */ >#include <linux/module.h> >#include <linux/kernel.h> >#include <linux/init.h> >#include <linux/uprobes.h> > >/* > * Usage: insmod wenji3x.ko pid=<pid> vaddr=<address> [verbose=0] > * where <pid> identifies the probed process and <address> is the virtual > * address of the probed instruction. > * > * insmod should report success (0) for the first registration attempt > * and failure (-16 = EBUSY) for the second and third attempts. The > * probe should work. > */ > >static int pid = 0; >module_param(pid, int, 0); >MODULE_PARM_DESC(pid, "pid"); > >static int verbose = 1; >module_param(verbose, int, 0); >MODULE_PARM_DESC(verbose, "verbose"); > >static long vaddr = 0; >module_param(vaddr, long, 0); >MODULE_PARM_DESC(vaddr, "vaddr"); > >static int nhits; >static struct uprobe usp; > >static void uprobe_handler(struct uprobe *u, struct pt_regs *regs) >{ > nhits++; > if (verbose) > printk(KERN_INFO "Hit #%d on probepoint at %#lx\n", > nhits, u->vaddr); >} > >int __init init_module(void) >{ > int ret, try, success = 0; > usp.pid = pid; > usp.vaddr = vaddr; > usp.handler = uprobe_handler; > printk(KERN_INFO "Registering uprobe on pid %d, vaddr %#lx\n", > usp.pid, usp.vaddr); > for (try = 1; try <= 3; try++) { > ret = register_uprobe(&usp); > printk(KERN_ERR "Try #%d: register_uprobe() returned %d\n", > try, ret); > if (ret == 0) > success++; > } > return (success ? 0 : ret); >} > >void __exit cleanup_module(void) >{ > printk(KERN_INFO "Unregistering uprobe on pid %d, vaddr %#lx\n", > usp.pid, usp.vaddr); > printk(KERN_INFO "Probepoint was hit %d times\n", nhits); > unregister_uprobe(&usp); >} >MODULE_LICENSE("GPL"); > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2007-07-09 8:19 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2007-07-06 8:03 multip register_uprobe Wenji Huang 2007-07-06 12:35 ` Srikar Dronamraju 2007-07-06 17:37 ` Jim Keniston 2007-07-06 18:50 ` Jim Keniston 2007-07-09 8:19 ` Wenji Huang
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).