Re: [PATCH 23/23] locktorture: Check the correct variable for allocation failure

[Paul E. McKenney]

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@xxxxxxxxxx>
> > > 
> > > 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@xxxxxxxxxx>
> > > Signed-off-by: Frederic Weisbecker <frederic@xxxxxxxxxx>
> > 
> > Reviewed-by: Paul E. McKenney <paulmck@xxxxxxxxxx>
> > 
> > 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;