Messages in this thread | | | Date | Mon, 20 Sep 1999 11:23:46 -0700 | From | "David S. Miller" <> | Subject | Re: [patch] __lock_sock race condition in 2.3.18* |
| |
Date: Mon, 20 Sep 1999 20:16:22 +0200 (CEST) From: Andrea Arcangeli <andrea@suse.de>
Woops, I missed that. If so I propose also this incremental patch to improve the scalability of the code:
--- 2.3.18ac6/include/net/sock.h Mon Sep 20 18:06:40 1999 +++ /tmp/sock.h Mon Sep 20 20:10:49 1999 @@ -672,8 +672,8 @@ (__sk)->lock.users = 0; \ if ((__sk)->backlog.tail != NULL) \ __release_sock(__sk); \ - wake_up(&((__sk)->lock.wq)); \ spin_unlock_bh(&((__sk)->lock.slock)); \ + wake_up(&((__sk)->lock.wq)); \ } while(0)
/* BH context may only use the following locking interface. */
No, you have decreased the scalability of this code especially in the case where some other CPU is spinning on the socket lock.
What happens now with your code is that the local cpu dirties the cache line containing the socket lock, now portions of the socket structure will travel over to the other cpu and leave the current cpu for the cache coherency transaction.
The other cpu will dirty the cache line by obtaining the spinlock.
Next the wait queue operations will be performed, moving a cache line'd portion of the sock structure back to the local processor.
Worse yet, if the local cpu dirties this cache line during the wake up operation it will be removed from the cache of the other processor due to cache coherency transaction rules.
In short, this suggested change will almost certainly make performance worse under high load.
Don't play with this code like this Andrea, Alexey and myself been thinking about these issues for months now. If you find the deadlock bug, thats great and please provide more details or even better a fix.
Later, David S. Miller davem@redhat.com
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
| |