Re: [PATCH RFC V4 06/10] jump_label: addarch_jump_label_transform_static() to optimise non-live code updates

[Peter Zijlstra]

On Wed, 2011-10-12 at 17:08 -0700, Jeremy Fitzhardinge wrote:
> From: Jeremy Fitzhardinge <jeremy.fitzhardinge@xxxxxxxxxx>
> 
> When updating a newly loaded module, the code is definitely not yet
> executing on any processor, so it can be updated with no need for any
> heavyweight synchronization.
> 
> This patch adds arch_jump_label_static() which is implemented as
> arch_jump_label_transform() by default, but architectures can override
> it if it avoids, say, a call to stop_machine().
> 
> Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@xxxxxxxxxx>
> Acked-by: Jason Baron <jbaron@xxxxxxxxxx>
> ---
>  include/linux/jump_label.h |    2 ++
>  kernel/jump_label.c        |   18 +++++++++++++++---
>  2 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
> index 12e804e..56594e4 100644
> --- a/include/linux/jump_label.h
> +++ b/include/linux/jump_label.h
> @@ -45,6 +45,8 @@ extern void jump_label_lock(void);
>  extern void jump_label_unlock(void);
>  extern void arch_jump_label_transform(struct jump_entry *entry,
>  				      enum jump_label_type type);
> +extern void arch_jump_label_transform_static(struct jump_entry *entry,
> +					     enum jump_label_type type);
>  extern int jump_label_text_reserved(void *start, void *end);
>  extern void jump_label_inc(struct jump_label_key *key);
>  extern void jump_label_dec(struct jump_label_key *key);
> diff --git a/kernel/jump_label.c b/kernel/jump_label.c
> index 059202d5..ff2028f 100644
> --- a/kernel/jump_label.c
> +++ b/kernel/jump_label.c
> @@ -104,6 +104,18 @@ static int __jump_label_text_reserved(struct jump_entry *iter_start,
>  	return 0;
>  }
>  
> +/* 
> + * Update code which is definitely not currently executing.
> + * Architectures which need heavyweight synchronization to modify
> + * running code can override this to make the non-live update case
> + * cheaper.
> + */
> +void __weak arch_jump_label_transform_static(struct jump_entry *entry,
> +					    enum jump_label_type type)
> +{
> +	arch_jump_label_transform(entry, type);	
> +}
> +
>  static void __jump_label_update(struct jump_label_key *key,
>  				struct jump_entry *entry,
>  				struct jump_entry *stop, int enable)
> @@ -135,8 +147,8 @@ static __init int jump_label_init(void)
>  		struct jump_label_key *iterk;
>  
>  		iterk = (struct jump_label_key *)(unsigned long)iter->key;
> -		arch_jump_label_transform(iter, jump_label_enabled(iterk) ?
> -					  JUMP_LABEL_ENABLE : JUMP_LABEL_DISABLE);
> +		arch_jump_label_transform_static(iter, jump_label_enabled(iterk) ?
> +						 JUMP_LABEL_ENABLE : JUMP_LABEL_DISABLE);
>  		if (iterk == key)
>  			continue;
>  
> @@ -208,7 +220,7 @@ void jump_label_apply_nops(struct module *mod)
>  		return;
>  
>  	for (iter = iter_start; iter < iter_stop; iter++)
> -		arch_jump_label_transform(iter, JUMP_LABEL_DISABLE);
> +		arch_jump_label_transform_static(iter, JUMP_LABEL_DISABLE);
>  }
>  

So I got myself a little confused wrt the early jump_label_apply_nops()
call and the MODULE_COMING notifiers.

It looks to me like jump_label_apply_nops() is called way early and is
in fact called before _any_ of the module code has had a chance of
running. However it simply NOPs out all jump_labels.

The jump_label_add_module() thing, which is ran on the MODULE_COMING
callback will then set up stuff and do the proper patch-up.

Now the only bit of the module text that can be ran between those two
calls appears to be the module argument parsing stuff, but since
jump_labels are non-functional it can't rely on them, so why do we do
the early patch up again?


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/