Message ID | 200910231936.16022.lgrijincu@ixiacom.com |
---|---|
State | Rejected, archived |
Delegated to: | David Miller |
Headers | show |
Lucian Adrian Grijincu a écrit : > Before udp hashes were converted to rcu in > udp: introduce struct udp_table and multiple spinlocks > 645ca708f936b2fbeb79e52d7823e3eb2c0905f8 > we stopped searching in list upon hitting the maximum score value (which is > 9). > > This got removed in the conversion to rcu. > I'm not sure whether this was intentional or it just slipped by. > > As far as I understand it this does not interfere with the lockless rcu: there > is another score check the result will have to pass and if it doesn't have a > score of 9 (which will be the value of badness) we'll just restart the lookup. > > Even if the node was deleted from the chain and reclaimed at a later time, if > at the second score test we have value 9 again, we can still return with this > result. > > Am I missing something? > > This was intentional. This never happens in practice and slowdown lookups. (To reach score 9, your UDP socket must be connected, and bound to a device) Most developpers dont even know UDP socket can be connected... We added large hashtables in commit f86dcc5aa8c7908f2c287e7a211228df599e3e71 (udp: dynamically size hash tables at boot time), so average chain length should be small anyway... -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c index d0d436d..69464b9 100644 --- a/net/ipv4/udp.c +++ b/net/ipv4/udp.c @@ -294,6 +294,11 @@ begin: sk_nulls_for_each_rcu(sk, node, &hslot->head) { score = compute_score(sk, net, saddr, hnum, sport, daddr, dport, dif); + if (score == 9) { + result = sk; + badness = score; + goto skip_end_of_nulls_check; + } if (score > badness) { result = sk; badness = score; @@ -307,6 +312,7 @@ begin: if (get_nulls_value(node) != hash) goto begin; +skip_end_of_nulls_check: if (result) { if (unlikely(!atomic_inc_not_zero(&result->sk_refcnt))) result = NULL;