Re: [PATCH v4 1/4] modules: split part of complete_formation() into prepare_coming_module()

[Josh Poimboeuf]

On Mon, Feb 08, 2016 at 11:50:21PM -0500, Jessica Yu wrote:
> Put all actions that are performed after module->state is set to
> MODULE_STATE_COMING in complete_formation() into a separate function
> prepare_coming_module(). This prepares for the removal of ftrace and
> livepatch module coming notifiers and instead the corresponding work will
> be done in prepare_coming_module(). This split will also allow for
> appropriate error handling.
> 
> Signed-off-by: Jessica Yu <jeyu@xxxxxxxxxx>

Reviewed-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>

> ---
>  kernel/module.c | 25 ++++++++++++++++++-------
>  1 file changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/kernel/module.c b/kernel/module.c
> index 9537da3..a174335 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -3366,6 +3366,14 @@ out_unlocked:
>  	return err;
>  }
>  
> +static int prepare_coming_module(struct module *mod)
> +{
> +
> +	blocking_notifier_call_chain(&module_notify_list,
> +				     MODULE_STATE_COMING, mod);
> +	return 0;
> +}
> +
>  static int complete_formation(struct module *mod, struct load_info *info)
>  {
>  	int err;
> @@ -3389,8 +3397,6 @@ static int complete_formation(struct module *mod, struct load_info *info)
>  	mod->state = MODULE_STATE_COMING;
>  	mutex_unlock(&module_mutex);
>  
> -	blocking_notifier_call_chain(&module_notify_list,
> -				     MODULE_STATE_COMING, mod);
>  	return 0;
>  
>  out:
> @@ -3512,13 +3518,17 @@ static int load_module(struct load_info *info, const char __user *uargs,
>  	if (err)
>  		goto ddebug_cleanup;
>  
> +	err = prepare_coming_module(mod);
> +	if (err)
> +		goto bug_cleanup;
> +
>  	/* Module is ready to execute: parsing args may do that. */
>  	after_dashes = parse_args(mod->name, mod->args, mod->kp, mod->num_kp,
>  				  -32768, 32767, mod,
>  				  unknown_module_param_cb);
>  	if (IS_ERR(after_dashes)) {
>  		err = PTR_ERR(after_dashes);
> -		goto bug_cleanup;
> +		goto coming_cleanup;
>  	} else if (after_dashes) {
>  		pr_warn("%s: parameters '%s' after `--' ignored\n",
>  		       mod->name, after_dashes);
> @@ -3527,7 +3537,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
>  	/* Link in to syfs. */
>  	err = mod_sysfs_setup(mod, info, mod->kp, mod->num_kp);
>  	if (err < 0)
> -		goto bug_cleanup;
> +		goto coming_cleanup;
>  
>  	/* Get rid of temporary copy. */
>  	free_copy(info);
> @@ -3537,15 +3547,16 @@ static int load_module(struct load_info *info, const char __user *uargs,
>  
>  	return do_init_module(mod);
>  
> + coming_cleanup:
> +	blocking_notifier_call_chain(&module_notify_list,
> +				     MODULE_STATE_GOING, mod);
> +
>   bug_cleanup:
>  	/* module_bug_cleanup needs module_mutex protection */
>  	mutex_lock(&module_mutex);
>  	module_bug_cleanup(mod);
>  	mutex_unlock(&module_mutex);
>  
> -	blocking_notifier_call_chain(&module_notify_list,
> -				     MODULE_STATE_GOING, mod);
> -
>  	/* we can't deallocate the module until we clear memory protection */
>  	module_disable_ro(mod);
>  	module_disable_nx(mod);
> -- 
> 2.4.3
> 

-- 
Josh