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;
