Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Date:   Tue, 10 Oct 2023 08:53:36 -0700
From:   "Paul E. McKenney" <paulmck@kernel.org>
To:     Dan Carpenter <dan.carpenter@linaro.org>
Cc:     Frederic Weisbecker <frederic@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Boqun Feng <boqun.feng@gmail.com>,
        Joel Fernandes <joel@joelfernandes.org>,
        Josh Triplett <josh@joshtriplett.org>,
        Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
        Neeraj Upadhyay <neeraj.upadhyay@amd.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Uladzislau Rezki <urezki@gmail.com>, rcu <rcu@vger.kernel.org>
Subject: Re: [PATCH 23/23] locktorture: Check the correct variable for
 allocation failure
Message-ID: <b9a976f6-c002-4e3d-8c6d-cef3953bd3a7@paulmck-laptop>
Reply-To: paulmck@kernel.org
References: <20231010115921.988766-1-frederic@kernel.org>
 <20231010115921.988766-24-frederic@kernel.org>
 <97ecfb49-e500-45f4-86bb-769b2d32e91c@paulmck-laptop>
 <beaba092-8545-43c9-9b65-6386f718cb01@kadam.mountain>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <beaba092-8545-43c9-9b65-6386f718cb01@kadam.mountain>
Precedence: bulk

On Tue, Oct 10, 2023 at 05:07:00PM +0300, Dan Carpenter wrote:
> On Tue, Oct 10, 2023 at 06:55:40AM -0700, Paul E. McKenney wrote:
> > On Tue, Oct 10, 2023 at 01:59:21PM +0200, Frederic Weisbecker wrote:
> > > From: Dan Carpenter <dan.carpenter@linaro.org>
> > > 
> > > There is a typo so this checks the wrong variable.  "chains" plural vs
> > > "chain" singular.  We already know that "chains" is non-zero.
> > > 
> > > Fixes: 7f993623e9eb ("locktorture: Add call_rcu_chains module parameter")
> > > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> > > Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> > 
> > Reviewed-by: Paul E. McKenney <paulmck@kernel.org>
> > 
> > A name change to increase the Hamming distance would of course also be
> > good, though less urgent.  ;-)
> 
> "Hamming distance" is such a great phrase.  I'm going to use that every
> time I complain about confusingly similar variable names going forward.

Glad you like it!

But the horrible thing is that I first heard that phrase back in
the 1970s, and I am the guilty party who created these particular
too-similar variable names.  (Why has the phrase fallen out of favor?
No idea, really, but one guess has to do with the fact that current
error-correcting codes must deal with different probabilities of different
bits flipping in different directions, so you would instead needs a
weirdly weighted variant of Hamming distance to accomplish anything with
modern error-correcting codes.)

But how about something like the following?

							Thanx, paul

------------------------------------------------------------------------

diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
index 991496afc0d9..48fd4562a754 100644
--- a/kernel/locking/locktorture.c
+++ b/kernel/locking/locktorture.c
@@ -126,7 +126,7 @@ struct call_rcu_chain {
 	struct rcu_head crc_rh;
 	bool crc_stop;
 };
-struct call_rcu_chain *call_rcu_chain;
+struct call_rcu_chain *call_rcu_chain_list;
 
 /* Forward reference. */
 static void lock_torture_cleanup(void);
@@ -1106,12 +1106,12 @@ static int call_rcu_chain_init(void)
 
 	if (call_rcu_chains <= 0)
 		return 0;
-	call_rcu_chain = kcalloc(call_rcu_chains, sizeof(*call_rcu_chain), GFP_KERNEL);
-	if (!call_rcu_chain)
+	call_rcu_chain_list = kcalloc(call_rcu_chains, sizeof(*call_rcu_chain_list), GFP_KERNEL);
+	if (!call_rcu_chain_list)
 		return -ENOMEM;
 	for (i = 0; i < call_rcu_chains; i++) {
-		call_rcu_chain[i].crc_stop = false;
-		call_rcu(&call_rcu_chain[i].crc_rh, call_rcu_chain_cb);
+		call_rcu_chain_list[i].crc_stop = false;
+		call_rcu(&call_rcu_chain_list[i].crc_rh, call_rcu_chain_cb);
 	}
 	return 0;
 }
@@ -1121,13 +1121,13 @@ static void call_rcu_chain_cleanup(void)
 {
 	int i;
 
-	if (!call_rcu_chain)
+	if (!call_rcu_chain_list)
 		return;
 	for (i = 0; i < call_rcu_chains; i++)
-		smp_store_release(&call_rcu_chain[i].crc_stop, true);
+		smp_store_release(&call_rcu_chain_list[i].crc_stop, true);
 	rcu_barrier();
-	kfree(call_rcu_chain);
-	call_rcu_chain = NULL;
+	kfree(call_rcu_chain_list);
+	call_rcu_chain_list = NULL;
 }
 
 static struct notifier_block lock_torture_stall_block;