Il 17/10/2013 12:10, Radim Krčmář ha scritto:
> Fix a bug when we free module memory while timer is pending by marking
> deferred static keys and flushing the timer on module unload.
>
> Also make static_key_rate_limit() useable more than once.
>
> Reproducer: (host crasher)
> modprobe kvm_intel
> (sleep 1; echo quit) \
> | qemu-kvm -kernel /dev/null -monitor stdio &
> sleep 0.5
> until modprobe -rv kvm_intel 2>/dev/null; do true; done
> modprobe -v kvm_intel
>
> Signed-off-by: Radim Krčmář <rkrc...@redhat.com>
> ---
> Very hacky; I've already queued generalizing ratelimit and applying it
> here, but there is still a lot to do on static keys ...
>
> include/linux/jump_label.h | 1 +
> kernel/jump_label.c | 17 ++++++++++++++++-
> 2 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
> index a507907..848bd15 100644
> --- a/include/linux/jump_label.h
> +++ b/include/linux/jump_label.h
> @@ -58,6 +58,7 @@ struct static_key {
> #ifdef CONFIG_MODULES
> struct static_key_mod *next;
> #endif
> + atomic_t deferred;
> };
>
> # include <asm/jump_label.h>
> diff --git a/kernel/jump_label.c b/kernel/jump_label.c
> index 297a924..7018042 100644
> --- a/kernel/jump_label.c
> +++ b/kernel/jump_label.c
> @@ -116,8 +116,9 @@ EXPORT_SYMBOL_GPL(static_key_slow_dec_deferred);
> void jump_label_rate_limit(struct static_key_deferred *key,
> unsigned long rl)
> {
> + if (!atomic_xchg(&key->key.deferred, 1))
> + INIT_DELAYED_WORK(&key->work, jump_label_update_timeout);
Can it actually happen that jump_label_rate_limit is called multiple
times? If so, this hunk alone would be a separate bugfix. I don't
think all the concurrency that you're protecting against can actually
happen, but in any case I'd just take the jump_label_lock() instead of
using atomics.
It's also not necessary to use a new field, since you can just check
key->timeout.
All this gives something like this for static_key_rate_limit_flush:
if (key->timeout) {
jump_label_lock();
if (key->enabled) {
jump_label_unlock();
flush_delayed_work(&dkey->work);
} else
jump_label_unlock();
}
Paolo
> key->timeout = rl;
> - INIT_DELAYED_WORK(&key->work, jump_label_update_timeout);
> }
> EXPORT_SYMBOL_GPL(jump_label_rate_limit);
>
> @@ -185,6 +186,14 @@ static enum jump_label_type jump_label_type(struct
> static_key *key)
> return JUMP_LABEL_DISABLE;
> }
>
> +static void static_key_rate_limit_flush(struct static_key *key)
> +{
> + struct static_key_deferred *dkey =
> + container_of(key, struct static_key_deferred, key);
> + if (atomic_read(&key->deferred))
> + flush_delayed_work(&dkey->work);
> +}
> +
> void __init jump_label_init(void)
> {
> struct jump_entry *iter_start = __start___jump_table;
> @@ -334,6 +343,12 @@ static void jump_label_del_module(struct module *mod)
>
> key = (struct static_key *)(unsigned long)iter->key;
>
> + /* We could also check if the static_key is used in its
> + * defining module and skip this flush then.
> + * (Rewrite of ratelimit in planned, so we don't care much)
> + */
> + static_key_rate_limit_flush(key);
> +
> if (__module_address(iter->key) == mod)
> continue;
>
>
--
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/
