On Wed, 30 Jan 2008, Peter Zijlstra wrote:

quoted text
>
> On Tue, 2008-01-29 at 22:15 -0500, Steven Rostedt wrote:
>
> > +int register_mcount_function(struct mcount_ops *ops)
> > +{
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&mcount_func_lock, flags);
> > +	ops->next = mcount_list;
> > +	/* must have next seen before we update the list pointer */
> > +	smp_wmb();
>
> That comment does not explain which race it closes; this is esp
> important as there is no paired barrier to give hints.

OK, fair enough. I'll explain it a bit more.

How's this:

 /*
  * We are entering ops into the mcount_list but another
  * CPU might be walking that list. We need to make sure
  * the ops->next pointer is valid before another CPU sees
  * the ops pointer included into the mcount_list.
  */

-- Steve

quoted text
>
> > +	mcount_list = ops;
> > +	/*
> > +	 * For one func, simply call it directly.
> > +	 * For more than one func, call the chain.
> > +	 */
> > +	if (ops->next == &mcount_list_end)
> > +		mcount_trace_function = ops->func;
> > +	else
> > +		mcount_trace_function = mcount_list_func;
> > +	spin_unlock_irqrestore(&mcount_func_lock, flags);
> > +
> > +	return 0;
> > +}
>
>
>
--
unsubscribe notice
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/