Christian Bell a écrit :

On Oct 28, 2008, at 1:37 PM, Eric Dumazet wrote:

quoted

-extern struct hlist_head udp_hash[UDP_HTABLE_SIZE];
-extern rwlock_t udp_hash_lock;
+struct udp_hslot {
+    struct hlist_head    head;
+    rwlock_t        lock;
+};

This structure should be aligned up to cacheline to reduce false sharing 
of more than one hslot.

Yes, I though about that. But : a full cache line is a waste of memory, and
choosing a power of two alignement is not easy because of 32bit/64bit arches,
and fact that sozepf(wrlock_t) can be > 4 if DEBUG

quoted

+    } else {
+        hslot = &udptable->hash[udp_hashfn(net, snum)];
+        write_lock_bh(&hslot->lock);
+        if (udp_lib_lport_inuse(net, snum, hslot, sk, saddr_comp))
+            goto fail;

The fail: label below should still unlock_bh when the above condition 
fails.

quoted

+    }
    inet_sk(sk)->num = snum;
    sk->sk_hash = snum;
    if (sk_unhashed(sk)) {
-        sk_add_node(sk, &udptable[udp_hashfn(net, snum)]);
+        sk_add_node(sk, &hslot->head);
        sock_prot_inuse_add(sock_net(sk), sk->sk_prot, 1);
    }
+    write_unlock_bh(&hslot->lock);
    error = 0;
fail:
-    write_unlock_bh(&udp_hash_lock);
    return error;
}

Good spoting, the write_unlock_bh(&hslot->lock); must be moved after the "fail:" label.

Thanks a lot