On Fri, Apr 21, 2017 at 12:08:26PM -0400, Jason Baron wrote:
> On 04/18/2017 06:32 AM, Peter Zijlstra wrote:
> > @@ -146,6 +154,7 @@ static void __static_key_slow_dec(struct
> > * returns is unbalanced, because all other static_key_slow_inc()
> > * instances block while the update is in progress.
> > */
> > + get_online_cpus();
> > if (!atomic_dec_and_mutex_lock(&key->enabled, &jump_label_mutex)) {
> > WARN(atomic_read(&key->enabled) < 0,
> > "jump label: negative count!\n");
>
> So the get and put can be unbalanced here since the above:
>
> 'if (!atomic_dec_and_mutex_lock(&key->enabled, &jump_label_mutex))'
>
> is followed by 'return;'. However, I see that the next patch removes this
> and so things are balanced again...
Duh.. right you are.
> > @@ -159,6 +168,7 @@ static void __static_key_slow_dec(struct
> > jump_label_update(key);
> > }
> > jump_label_unlock();
> > + put_online_cpus();
> > }
> >
> > static void jump_label_update_timeout(struct work_struct *work)
> > @@ -592,6 +602,10 @@ jump_label_module_notify(struct notifier
> >
> > switch (val) {
> > case MODULE_STATE_COMING:
> > + /*
> > + * XXX do we need get_online_cpus() ? the module isn't
> > + * executable yet, so nothing should be looking at our code.
> > + */
>
> Since we're just updating the table of places we potentially need to patch,
> but not actually doing any patching, we should not need get_online_cpus()
> here...so in attempt to reduce confusion I would remove this.
Thanks for confirming it is indeed not required. Will make it go away.
