On Tue, 2014-01-21 at 20:06 +0100, Ingo Molnar wrote: > * Tim Chen <tim.c.chen@xxxxxxxxxxxxxxx> wrote: > > > On Tue, 2014-01-21 at 11:41 +0100, Ingo Molnar wrote: > > > * Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote: > > > > > > > On Mon, Jan 20, 2014 at 05:24:31PM -0800, Tim Chen wrote: > > > > > +EXPORT_SYMBOL_GPL(mcs_spin_lock); > > > > > +EXPORT_SYMBOL_GPL(mcs_spin_unlock); > > > > > > > > Do we really need the EXPORTs? The only user so far is mutex and that's > > > > core code. The other planned users are rwsems and rwlocks, for both it > > > > would be in the slow path, which is also core code. > > > > > > > > We should generally only add EXPORTs once theres a need. > > > > > > In fact I'd argue the hot path needs to be inlined. > > > > > > We only don't inline regular locking primitives because it would blow > > > up the kernel's size in too many critical places. > > > > > > But inlining an _internal_ locking implementation used in just a > > > handful of places is a no-brainer... > > > > The original mspin_lock primitive from which mcs_spin_lock was > > derived has an explicit noinline annotation. The comment says that > > it is so that perf can properly account for time spent in the lock > > function. So it wasn't inlined in previous kernels when we started. > > Not sure what comment that was, but it's not a valid argument: > profiling and measurement is in almost all cases secondary to any > performance considerations! > > If we keep it out of line then we want to do it only if it's faster > that way. > > > For the time being, I'll just remove the EXPORT. If people feel > > that inline is the right way to go, then we'll leave the function in > > mcs_spin_lock.h and not create mcs_spin_lock.c. > > Well, 'people' could be you, the person touching the code? This is > really something that is discoverable: look at the critical path in > the inlined and the out of line case, and compare the number of > instructions. This can be done based on disassembly of the affected > code. Okay, will make it inline function and drop the move of to mcs_spin_lock.c Tim > > Thanks, > > Ingo -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>