Re: [PATCH v4 3.0-rc2-tip 4/22] 4: Uprobes: register/unregister probes.

[Peter Zijlstra <peterz@infradead.org>]

On Tue, 2011-06-07 at 18:29 +0530, Srikar Dronamraju wrote:
> +/*
> + * There could be threads that have hit the breakpoint and are entering the
> + * notifier code and trying to acquire the uprobes_treelock. The thread
> + * calling delete_uprobe() that is removing the uprobe from the rb_tree can
> + * race with these threads and might acquire the uprobes_treelock compared
> + * to some of the breakpoint hit threads. In such a case, the breakpoint hit
> + * threads will not find the uprobe. Finding if a "trap" instruction was
> + * present at the interrupting address is racy. Hence provide some extra
> + * time (by way of synchronize_sched() for breakpoint hit threads to acquire
> + * the uprobes_treelock before the uprobe is removed from the rbtree.
> + */

'some' extra time doesn't really sound convincing to me. Either it is
sufficient to avoid the race or it is not. It reads to me like: we add a
delay so that the race mostly doesn't occur. Not good ;-)

> +static void delete_uprobe(struct uprobe *uprobe)
> +{
> +       unsigned long flags;
> +
> +       synchronize_sched();
> +       spin_lock_irqsave(&uprobes_treelock, flags);
> +       rb_erase(&uprobe->rb_node, &uprobes_tree);
> +       spin_unlock_irqrestore(&uprobes_treelock, flags);
> +       iput(uprobe->inode);
> +} 

Also what are the uprobe lifetime rules here? Does it still exist after
this returns?

The comment in del_consumer() that says: 'drop creation ref' worries me
and makes me thing that is the last reference around and the uprobe will
be freed right there, which clearly cannot happen since its not yet
removed from the RB-tree.


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>