On Sun, Jul 09, 2017 at 11:38:50 -1000, Richard Henderson wrote:
> On 07/08/2017 09:50 PM, Emilio G. Cota wrote:
(snip)
> I think it would be better to have a tb_htable_lookup_or_insert function,
> which performs the insert iff a matching object isn't already there,
> returning the entry which *is* there in either case.
qht_insert behaves exactly like this, except that it returns a bool.
But we could make it return a void *.
> The hash table already has per-bucket locking. So here you're taking 3
> locks (tb_lock, bucket lock for lookup, bucket lock for insert) instead of
> just taking the bucket lock once. And, recall, the htable is designed such
> that the buckets do not contend often, so you ought to be eliminating most
> of the contention that you're seeing on tb_lock.
Minor nit: the lookup is just a seqlock_read, so it's not really a lock.
Your point is correct though. Performing a lookup here (that qht_insert
will do anyway) is wasteful. I didn't look further into this because I
thought getting rid of tb_lock here (due to tb_link_page) wasn't trivial.
But I'll look into it; if we manage to just have the lock in qht_insert,
we'll get great scalability.
E.
