Re: [PATCH] EXP hashtorture.h: Avoid sporadic SIGSEGV in hash_bkt_rcu

From: "Paul E. McKenney" <paulmck@linux.ibm.com>
To: Akira Yokosawa <akiyks@gmail.com>
Cc: perfbook@vger.kernel.org
Subject: Re: [PATCH] EXP hashtorture.h: Avoid sporadic SIGSEGV in hash_bkt_rcu
Date: Wed, 2 Jan 2019 11:18:22 -0800	[thread overview]
Message-ID: <20190102191822.GA30360@linux.ibm.com> (raw)
In-Reply-To: <20190102171848.GA4170@linux.ibm.com>

On Wed, Jan 02, 2019 at 09:18:48AM -0800, Paul E. McKenney wrote:
> On Thu, Jan 03, 2019 at 12:02:56AM +0900, Akira Yokosawa wrote:
> > On 2019/01/01 10:00:25 -0800, Paul E. McKenney wrote:

[ . . . ]

> > There is one more thing I've noticed with "hash_resize --schroedinger".
> > *Without* resizing enabled, it says:
> > 
> >     $ ./hash_resize --schroedinger
> >     nlookups: 91373 91373  ncats: 0  nadds: 5  ndels: 6  duration: 10.851
> >     ns/read: 118.755  ns/update: 986455
> > 
> > This means that all the lookups failed. OTOH, hash_bkt_rcu works as expected
> > as follows:
> > 
> >     $ ./hash_bkt_rcu --schroedinger
> >     nlookups: 56064 28004  ncats: 0  nadds: 5  ndels: 5  duration: 10.373
> >     ns/read: 185.021  ns/update: 1.0373e+06
> > 
> > (ns/read looks slow because compiler optimization is disabled.)
> > 
> > There seems to be some mismatch in hash/key handling of hash_resize.c --
> > hashtorture.h combination. I've not yet figured out the cause, though.
> 
> The short story is that I am working to return the locking state
> from hashtab_lock_mod() for use by hashtab_add(), hashtab_del(),
> and hashtab_unlock_mod().  Also, the first resize carries out some
> "interesting" state changes that might need to be reflected in
> initialization.
> 
> But yes, not one of my best efforts...

To be a bit less cryptic, the problem is that the non-resizable hash
tables completely delegate hashing to the caller.  This is not viable
for resizable hash tables (which were indeed added later) because the
resizing is internal to the hash table, yet needs to know the hash
function.  And currently hash_resize.c and hashtorture.h agree on the
hash function except for the --schroedinger case.  So zoo_lookup()
and zoo_add() won't be looking at the same buckets except by accident.
And a rather low-probability accident at that.

So I remove "hash_private", which was intended to allow hash-function
perturbation, but is not used.  Then I can pass the hash function into
hashtab_alloc(), where all but hash_resize can ignore it.  Then I continue
the locking/bucket-selection cleanup.

And again, thank you for finding these problems!

							Thanx, Paul