Messages in this thread Patch in this message | | | Subject | Re: [PATCH 1/3] jump_label: Pull get_online_cpus() into generic code | From | Jason Baron <> | Date | Fri, 21 Apr 2017 12:08:26 -0400 |
| |
On 04/18/2017 06:32 AM, Peter Zijlstra wrote: > This change does two things; it moves the get_online_cpus() call into > generic code, with the aim of later providing some static_key ops that > avoid it. > > And as a side effect it inverts the relation between cpu_hotplug_lock > and jump_label_mutex. > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > ---
...
> @@ -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...
> @@ -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,
-Jason
| |