linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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
Date: Tue, 2 Feb 2016 16:23:31 -0800	[thread overview]
Message-ID: <20160203002331.GA3385@linux-uzut.site> (raw)
In-Reply-To: <20160202064635.GH6719@linux.vnet.ibm.com>

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

  reply	other threads:[~2016-02-03  0:23 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-28  4:25 [PATCH v2] locktorture: Fix NULL pointer when torture_type is invalid Kefeng Wang
2016-01-30  2:46 ` Kefeng Wang
2016-01-31  0:27   ` Paul E. McKenney
2016-01-31 22:17     ` Davidlohr Bueso
2016-02-01  2:25       ` Kefeng Wang
2016-02-01  3:02         ` Davidlohr Bueso
2016-02-01  3:28           ` Kefeng Wang
2016-02-02  6:46             ` Paul E. McKenney
2016-02-03  0:23               ` Davidlohr Bueso [this message]
2016-03-02 19:55                 ` Davidlohr Bueso
2016-03-02 21:12                   ` Paul E. McKenney
2016-03-03  1:37                     ` Kefeng Wang
2016-03-03  4:31                       ` Kefeng Wang
2016-03-03  8:36                         ` Davidlohr Bueso
2016-03-04 18:41                           ` Davidlohr Bueso
2016-03-07  2:00                           ` Kefeng Wang
2016-03-07  5:40                             ` Davidlohr Bueso
2016-03-07  7:02                               ` Kefeng Wang
2016-03-07 13:37                                 ` Paul E. McKenney
2016-03-08  2:10                                   ` Kefeng Wang
2016-03-08 19:51                                     ` Paul E. McKenney
2016-03-03 14:14                       ` Paul E. McKenney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160203002331.GA3385@linux-uzut.site \
    --to=dave@stgolabs.net \
    --cc=guohanjun@huawei.com \
    --cc=josh@joshtriplett.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=wangkefeng.wang@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).