Messages in this thread | | | Date | Wed, 26 Aug 1998 16:18:26 -0700 (PDT) | From | Linus Torvalds <> | Subject | Re: Linux 2.1.118 SMP problem |
| |
On Wed, 26 Aug 1998, Linus Torvalds wrote: > > What lock_sock() should really do is > > atomic_inc(&sk->sock_readers); > synchronize_bh(); > > instead of getting the main interrupt lock. The end result is the same, > but at least this way it's clearer what it tries to do (and I can remove > the "You don't want to know" comment ;)
When I did this it became fairly obvious that the old code _was_ broken.
I initially tried to be cheezy, and do the "I know this is not strictly right, but it's the smaller change" version of the above, which didn't actually make "sock_readers" be atomic, on the assumption that everybody who tried to lock the socket were inside the kernel lock in a process context.
That very quickly showed itself to not be true, with various strange and fun things happening to TCP. And THAT in turn means that the old code was always buggy, because while the sock_readers increment was protected by the single irq lock, the decrement wasn't protected by anything at all.
It probably worked well enough because the "cli/sti" thing that protected the increment also acted as a fairly strong barrier, and as such it was very unlikely to have races later on during the decrement, but it certainly looks like the above change to the locking logic is not only the technically correct one, it also fixes a real bug.
I'll be testing this out some more, but that could have been a cause of some interesting problems on SMP..
Linus
- 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.altern.org/andrebalsa/doc/lkml-faq.html
| |