linux-kernel - Re: [PATCH 2/3] rhashtable: don't hold lock on first table throughout insertion.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 20 Mar 2019 13:40:21 +0800
From:   Herbert Xu <herbert@...dor.apana.org.au>
To:     NeilBrown <neilb@...e.com>
Cc:     Thomas Graf <tgraf@...g.ch>, netdev@...r.kernel.org,
        "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] rhashtable: don't hold lock on first table
 throughout insertion.

On Fri, Mar 15, 2019 at 05:46:37PM +1100, NeilBrown wrote:
>
> From: NeilBrown <neilb@...e.com>
> Date: Mon, 25 Jun 2018 08:09:19 +1000
> Subject: [PATCH] rhashtable: don't hold lock on first table throughout
>  insertion.
> 
> rhashtable_try_insert() currently holds a lock on the bucket in
> the first table, while also locking buckets in subsequent tables.
> This is unnecessary and looks like a hold-over from some earlier
> version of the implementation.
> 
> As insert and remove always lock a bucket in each table in turn, and
> as insert only inserts in the final table, there cannot be any races
> that are not covered by simply locking a bucket in each table in turn.
> 
> When an insert call reaches that last table it can be sure that there
> is no matchinf entry in any other table as it has searched them all, and
> insertion never happens anywhere but in the last table.  The fact that
> code tests for the existence of future_tbl while holding a lock on
> the relevant bucket ensures that two threads inserting the same key
> will make compatible decisions about which is the "last" table.
> 
> This simplifies the code and allows the ->rehash field to be
> discarded.
> 
> We still need a way to ensure that a dead bucket_table is never
> re-linked by rhashtable_walk_stop().  This can be achieved by calling
> call_rcu() inside the locked region, and checking with
> rcu_head_after_call_rcu() in rhashtable_walk_stop() to see if the
> bucket table is empty and dead.
> 
> Signed-off-by: NeilBrown <neilb@...e.com>
> ---
>  include/linux/rhashtable.h | 13 ------------
>  lib/rhashtable.c           | 52 ++++++++++++++--------------------------------
>  2 files changed, 16 insertions(+), 49 deletions(-)

Acked-by: Herbert Xu <herbert@...dor.apana.org.au>
-- 
Email: Herbert Xu <herbert@...dor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt