This is the mail archive of the
systemtap@sourceware.org
mailing list for the systemtap project.
Re: [PATCH v2 2.6.38-rc8-tip 5/20] 5: Uprobes: register/unregister probes.
- From: Peter Zijlstra <peterz at infradead dot org>
- To: Srikar Dronamraju <srikar at linux dot vnet dot ibm dot com>
- Cc: Steven Rostedt <rostedt at goodmis dot org>, Thomas Gleixner <tglx at linutronix dot de>, Ingo Molnar <mingo at elte dot hu>, Linux-mm <linux-mm at kvack dot org>, Arnaldo Carvalho de Melo <acme at infradead dot org>, Linus Torvalds <torvalds at linux-foundation dot org>, Christoph Hellwig <hch at infradead dot org>, Masami Hiramatsu <masami dot hiramatsu dot pt at hitachi dot com>, Ananth N Mavinakayanahalli <ananth at in dot ibm dot com>, Oleg Nesterov <oleg at redhat dot com>, LKML <linux-kernel at vger dot kernel dot org>, SystemTap <systemtap at sources dot redhat dot com>, Jim Keniston <jkenisto at linux dot vnet dot ibm dot com>, Roland McGrath <roland at hack dot frob dot com>, Andi Kleen <andi at firstfloor dot org>, Andrew Morton <akpm at linux-foundation dot org>, "Paul E. McKenney" <paulmck at linux dot vnet dot ibm dot com>
- Date: Tue, 15 Mar 2011 19:15:49 +0100
- Subject: Re: [PATCH v2 2.6.38-rc8-tip 5/20] 5: Uprobes: register/unregister probes.
- References: <20110314133403.27435.7901.sendpatchset@localhost6.localdomain6> <20110314133454.27435.81020.sendpatchset@localhost6.localdomain6> <alpine.LFD.2.00.1103151439400.2787@localhost6.localdomain6> <20110315171536.GA24254@linux.vnet.ibm.com> <1300211262.9910.295.camel@gandalf.stny.rr.com> <1300211411.2203.290.camel@twins> <20110315180423.GA10429@linux.vnet.ibm.com>
On Tue, 2011-03-15 at 23:34 +0530, Srikar Dronamraju wrote:
> * Peter Zijlstra <peterz@infradead.org> [2011-03-15 18:50:11]:
>
> > On Tue, 2011-03-15 at 13:47 -0400, Steven Rostedt wrote:
> > > On Tue, 2011-03-15 at 22:45 +0530, Srikar Dronamraju wrote:
> > > > > > + }
> > > > > > + list_for_each_entry_safe(mm, tmpmm, &tmp_list, uprobes_list) {
> > > > > > + down_read(&mm->mmap_sem);
> > > > > > + if (!install_uprobe(mm, uprobe))
> > > > > > + ret = 0;
> > > > >
> > > > > Installing it once is success ?
> > > >
> > > > This is a little tricky. My intention was to return success even if one
> > > > install is successful. If we return error, then the caller can go
> > > > ahead and free the consumer. Since we return success if there are
> > > > currently no processes that have mapped this inode, I was tempted to
> > > > return success on atleast one successful install.
> > >
> > > What about an all or nothing approach. If one fails, remove all that
> > > were installed, and give up.
> >
> > That sounds like a much saner semantic and is what we generally do in
> > the kernel.
>
> One of the install_uprobe could be failing because the process was
> almost exiting, something like there was no mm->owner. Also lets
> assume that the first few install_uprobes go thro and the last
> install_uprobe fails. There could be breakpoint hits corresponding to
> the already installed uprobes that get displayed. i.e all
> breakpoint hits from the first install_uprobe to the time we detect a
> failed a install_uprobe and revert all inserted breakpoints will be
> shown as being captured.
I think you can gracefully deal with the exit case and simply ignore
that one. But you cannot let arbitrary installs fail and still report
success, that gives very weak and nearly useless semantics.
> Also register_uprobe will only install probes for processes that are
> actively and have mapped the inode. However install_uprobe called from
> uprobe_mmap which also registers the probe can fail. Now should we take
> the same yardstick and remove all the probes corresponding to the
> successful register_uprobe?
No, if uprobe_mmap() fails we should fail the mmap(), that also
guarantees consistency.