From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sowmini Varadhan Subject: Re: [Patch net] rds: mark bound socket with SOCK_RCU_FREE Date: Mon, 10 Sep 2018 20:04:37 -0400 Message-ID: <20180911000437.GK4668@oracle.com> References: <20180910222422.19470-1-xiyou.wangcong@gmail.com> <20180910233007.GJ4668@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Santosh Shilimkar , Linux Kernel Network Developers , rds-devel@oss.oracle.com To: Cong Wang Return-path: Received: from userp2120.oracle.com ([156.151.31.85]:47904 "EHLO userp2120.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726198AbeIKFBS (ORCPT ); Tue, 11 Sep 2018 01:01:18 -0400 Content-Disposition: inline In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: 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