Re: [PATCH v2] locktorture: Fix NULL pointer when torture_type is invalid

[Davidlohr Bueso]

On Tue, 02 Feb 2016, Davidlohr Bueso wrote:

I've just hit this issue myself and remembered this thread :)

Paul, folks, does the below patch look reasonable to you? If so
I can properly resend. thanks.

> On Mon, 01 Feb 2016, Paul E. McKenney wrote:
>
> > On Mon, Feb 01, 2016 at 11:28:07AM +0800, Kefeng Wang wrote:
>
> > > Just like I mentioned before, keep consistent with rcutorture???
>
> Because rcutorture does it doesn't mean locktorture has to do it ;)
> In any case, I'd suggest the same be done for rcutorture.
>
> [...]
>
> > Hmmm...   If nothing happened, then I agree that it makes sense not to
> > print any statistics.  But if some testing actually was carried out, then
> > we really need to print the statistics.
>
> Right, so how about the following? It introduces an early cleanup helper
> that all it does is do torture specific cleanups. I don't really love the
> begin/end calls there, but it's not the end of the world and it seems better
> than a more messier refactoring. ie, I had also considered adding an 'early'
> flag to lock_torture_cleanup() such that we can enable it for this bogus param
> scenario, but seems over complicating things and we also call it for such a
> small issue.
>
> Thanks,
> Davidlohr
>
>
> diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
> index 8ef1919..05e2649 100644
> --- a/kernel/locking/locktorture.c
> +++ b/kernel/locking/locktorture.c
> @@ -741,6 +741,19 @@ lock_torture_print_module_parms(struct lock_torture_ops *cur_ops,
> 		 onoff_interval, onoff_holdoff);
> }
> +/*
> + * Indicates early cleanup, meaning that the test has not run,
> + * such as when passing bogus args when loading the module. As
> + * such, only perform the underlying torture-specific cleanups,
> + * and avoid anything related to locktorture.
> + */
> +static inline void lock_torture_early_cleanup(void)
> +{
> +	if (torture_cleanup_begin())
> +		return;
> +	torture_cleanup_end();
> +}
> +
> static void lock_torture_cleanup(void)
> {
> 	int i;
> @@ -811,8 +824,10 @@ static int __init lock_torture_init(void)
> 		for (i = 0; i < ARRAY_SIZE(torture_ops); i++)
> 			pr_alert(" %s", torture_ops[i]->name);
> 		pr_alert("\n");
> -		firsterr = -EINVAL;
> -		goto unwind;
> +
> +		torture_init_end();
> +		lock_torture_early_cleanup();
> +		return -EINVAL;
> 	}
> 	if (cxt.cur_ops->init)
> 		cxt.cur_ops->init();