lkml.org 
[lkml]   [2017]   [Apr]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH 1/3] jump_label: Pull get_online_cpus() into generic code
From
Date
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

\
 
 \ /
  Last update: 2017-04-21 19:02    [W:0.075 / U:0.180 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site