Re: [PATCH v8 3.2.0-rc5 1/9] uprobes: Install and removebreakpoints.

[Srikar Dronamraju]

* Peter Zijlstra <peterz@xxxxxxxxxxxxx> [2012-01-04 17:49:48]:

> On Fri, 2011-12-16 at 17:58 +0530, Srikar Dronamraju wrote:
> > +static void __unregister_uprobe(struct uprobe *uprobe)
> > +{
> > +       if (!register_for_each_vma(uprobe, false))
> > +               delete_uprobe(uprobe);
> > +
> > +       /* TODO : cant unregister? schedule a worker thread */
> > +} 
> 
> I was about to suggest we merge it, but we really can't with a hole that
> size..
> 

On failure of unregister due to low memory condition:
	- uprobe is left in the rbtree.  So subsequent probe hits can
	  still refer the nodes.

	- UPROBES_RUN_HANDLER flag still gets reset unconditionally. So
	  handlers will not run on subsequent probe hits that correspond
	  to this uprobe.

	- consumers for the uprobe is NULL, so mmap_uprobe will not
	  insert new breakpoints which correspond to this uprobe until
	  or unless another consumer gets added for the same probe.

	- If a new consumer gets added for this probe, we reuse the
	  uprobe struct.
	
So in the highly unlikely case of uprobes not being able to unregister
cleanly because of low memory conditions, existing tasks that have this
probe in their address space will incur an extra cost of handling
exceptions.

However scheduling a kworker thread (or removing the underlying
breakpoint if we detect the probe doesnt have UPROBES_RUN_HANDLER flag
set) will reduce this overhead. 

Since we are looking at an extra overhead and no change in behaviour,
should this be a reason to stop merging this feature?

-- 
Thanks and Regards
Srikar

--
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/