On Wed, Jul 10, 2013 at 04:39:02PM +0200, Ingo Molnar wrote: > > * Peter Zijlstra <pet...@infradead.org> wrote: > > > Hi, > > > > I just stumbled over commit 582b336ec2 and had a massive WTF moment. > > > > The Changelog -- empty! > > The Implementation -- complete crap! > > > > fail^2. > > > > A git grep later I find its x86_64 paravirt only.. for this we add > > unconditional crap to the scheduler? > > > > At the _very_ least this should have been wrapped in a static_key and > > the changelog should have given some clue as to the what and why of this > > code. > > My bad for letting it slip through... > > Marcelo, mind implementing the suggestions from Peter?
So ideally we'd kill the entire notifier, notifiers make it far too easy for others to use -- and I don't want silent users of stuff like this. Ideally I'd see a direct callback: pvclock_migration_callback(t, new_cpu); That compiles to 'do { } while (0);' for kernels without CONFIG_PARAVIRT. Then for paravirt it would use a static_key to get a real callback only if there's a pvclock user. Something like: #define pvclock_migration_callback(_t, _cpu) \ do { \ if (static_key_false(&pvclock_key)) \ __pvclock_migration_callback((_t), (_cpu)); \ } while (0) NOTE: static_key_false() doesn't test false, it assumes the default is false and makes the function call the out-of-line jump -- horridly confusing function name. Your pvclock muck would do: static_key_slow_inc() -- for every new user and, static_key_slow_dec() -- for every user gone. Its a reference count scheme, so that when there's no users there is only a 5 byte nop. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/