Re: [PATCH 02/22 -v7] Add basic support for gcc profiler instrumentation

From: Steven Rostedt
Date: Wed Jan 30 2008 - 08:08:40 EST




On Wed, 30 Jan 2008, Peter Zijlstra wrote:

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

>
> > + 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;
> > +}
>
>
>
--
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/