Date: Wed, 2 Mar 2016 11:55:43 -0800
From: Davidlohr Bueso <dave@stgolabs.net>
To: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: Kefeng Wang <wangkefeng.wang@huawei.com>, linux-kernel@vger.kernel.org,
        peterz@infradead.org, mingo@redhat.com,
        Josh Triplett <josh@joshtriplett.org>,
        "Guohanjun (Hanjun Guo)" <guohanjun@huawei.com>
Subject: Re: [PATCH v2] locktorture: Fix NULL pointer when torture_type is
 invalid
Message-ID: <20160302195543.GA12593@linux-uzut.site>
References: <1453955159-23216-1-git-send-email-wangkefeng.wang@huawei.com>
 <56AC2421.7020006@huawei.com>
 <20160131002721.GI6719@linux.vnet.ibm.com>
 <20160131221736.GB16147@linux-uzut.site>
 <56AEC21A.5010107@huawei.com>
 <20160201030235.GC16147@linux-uzut.site>
 <56AED0C7.7050505@huawei.com>
 <20160202064635.GH6719@linux.vnet.ibm.com>
 <20160203002331.GA3385@linux-uzut.site>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
In-Reply-To: <20160203002331.GA3385@linux-uzut.site>
User-Agent: Mutt/1.5.21 (2010-09-15)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2331
Lines: 71

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();