* Mathieu Desnoyers (mathieu.desnoyers@xxxxxxxxxxxx) wrote: > * Sasha Levin (levinsasha928@xxxxxxxxx) wrote: > [...] > > @@ -158,34 +159,21 @@ static int dlm_allow_conn; > > static struct workqueue_struct *recv_workqueue; > > static struct workqueue_struct *send_workqueue; > > > > -static struct hlist_head connection_hash[CONN_HASH_SIZE]; > > +static struct hlist_head connection_hash[CONN_HASH_BITS]; > > static DEFINE_MUTEX(connections_lock); > > static struct kmem_cache *con_cache; > > > > static void process_recv_sockets(struct work_struct *work); > > static void process_send_sockets(struct work_struct *work); > > > > - > > -/* This is deliberately very simple because most clusters have simple > > - sequential nodeids, so we should be able to go straight to a connection > > - struct in the array */ > > -static inline int nodeid_hash(int nodeid) > > -{ > > - return nodeid & (CONN_HASH_SIZE-1); > > -} > > There is one thing I dislike about this change: you remove a useful > comment. It's good to be informed of the reason why a direct mapping > "value -> hash" without any dispersion function is preferred here. And now that I come to think of it: you're changing the behavior : you will now use a dispersion function on the key, which goes against the intent expressed in this comment. It might be good to change hash_add(), hash_add_rcu(), hash_for_each_possible*() key parameter for a "hash" parameter, and let the caller provide the hash value computed by the function they like as parameter, rather than enforcing hash_32/hash_64. Thoughts ? Thanks, Mathieu -- Mathieu Desnoyers Operating System Efficiency R&D Consultant EfficiOS Inc. http://www.efficios.com -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html