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