[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20180911000437.GK4668@oracle.com>
Date: Mon, 10 Sep 2018 20:04:37 -0400
From: Sowmini Varadhan <sowmini.varadhan@...cle.com>
To: Cong Wang <xiyou.wangcong@...il.com>
Cc: Santosh Shilimkar <santosh.shilimkar@...cle.com>,
Linux Kernel Network Developers <netdev@...r.kernel.org>,
rds-devel@....oracle.com
Subject: Re: [Patch net] rds: mark bound socket with SOCK_RCU_FREE
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
Powered by blists - more mailing lists