Re: [v2 PATCH 7/10] rhashtable: Disable automatic shrinking

On 03/23/15 at 08:29pm, Herbert Xu wrote:

On Mon, Mar 23, 2015 at 08:37:12AM +0000, Thomas Graf wrote:

quoted

I think rhashtable_shrink() should fetch ht->tbl in an RCU section to
cheaply get the current table size and only do the allocation and take
the lock if the table size warrants for shrinking.

Well you should never invoke rhashtable_shrink unless you actually
wanted to shrink.  So this is something that you should have checked
before rhashtable_shrink is called.

How? The calculation of the table size is embedded in
rhashtable_shrink(). Should every user have a copy of that
calculation algorithm?

Why not just:

	unlikely(ht->p.shrink && rht_shrink_below_30(..))

If you really care about that additional conditional we
can also add:

static inline int rhashtable_remove_and_shrink()
{
        int err;

        rcu_read_lock();

	tbl = rht_dereference_rcu(ht->tbl, ht);

        err = rhashtable_remove_fast();
        if (unlikely(!err && rht_shrink_below_30(ht, tbl)))
                schedule_work(&ht->run_work);

	rcu_read_unlock();

	return err;
}

I just think it's wrong to rip out all the shrinking logic and
require every single user to re-add its own copy.