Re: [PATCH v2 2.6.38-rc8-tip 5/20] 5: Uprobes: register/unregisterprobes.

[Peter Zijlstra]

On Tue, 2011-03-15 at 23:34 +0530, Srikar Dronamraju wrote:
> * Peter Zijlstra <peterz@xxxxxxxxxxxxx> [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.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/