Re: [Patch net] rds: mark bound socket with SOCK_RCU_FREE

[Sowmini Varadhan <sowmini.varadhan@oracle.com>]

On (09/10/18 16:51), Cong Wang wrote:
> 
>         __rds_create_bind_key(key, addr, port, scope_id);
> -       rs = rhashtable_lookup_fast(&bind_hash_table, key, ht_parms);
> +       rcu_read_lock();
> +       rs = rhashtable_lookup(&bind_hash_table, key, ht_parms);
>         if (rs && !sock_flag(rds_rs_to_sk(rs), SOCK_DEAD))
>                 rds_sock_addref(rs);
>         else
>                 rs = NULL;
> +       rcu_read_unlock();

aiui, the rcu_read lock/unlock is only useful if the write
side doing destructive operations  does something to make sure readers
are done before doing the destructive opertion. AFAIK, that does
not exist for rds socket management today


> Although sock release path is not a very hot path, but blocking
> it isn't a good idea either, especially when you can use call_rcu(),
> which has the same effect.
> 
> I don't see any reason we should prefer synchronize_rcu() here.

Usually correctness (making sure all readers are done, before nuking a
data structure) is a little bit more important than perforamance, aka
"safety before speed" is what I've always been taught.  

Clearly, your mileage varies. As you please.

--Sowmini