Date: Fri, 24 May 2002 15:18:40 +0900 From: Seigo Tanimura <tanimura@r.dl.itc.u-tokyo.ac.jp> To: John Baldwin <jhb@FreeBSD.ORG> Cc: Jeffrey Hsu <hsu@FreeBSD.ORG>, smp@FreeBSD.ORG Subject: Re: socket locks Message-ID: <200205240618.g4O6Ie3i020960@rina.r.dl.itc.u-tokyo.ac.jp> In-Reply-To: <XFMail.20020521103913.jhb@FreeBSD.org> References: <0GWF00DL3Q1GXZ@mta6.snfc21.pbi.net> <XFMail.20020521103913.jhb@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 21 May 2002 10:39:13 -0400 (EDT), John Baldwin <jhb@FreeBSD.ORG> said: jhb> On 21-May-2002 Jeffrey Hsu wrote: >> > tanimura 2002/05/19 22:41:09 PDT >> > Modified files: >> > (too many files) >> > Log: >> > Lock down a socket, milestone 1. >> > >> > Reviewed by: alfred >> >> This patch mechanically adds a lot of locks around uses of socket fields. >> For example, in tcp_output.c: >> >> @@ -889,8 +897,11 @@ send: >> #ifdef IPSEC >> ipsec_setsocket(m, so); >> #endif /*IPSEC*/ >> + SOCK_LOCK(so); >> + soopts = (so->so_options & SO_DONTROUTE); >> + SOCK_UNLOCK(so); >> error = ip_output(m, tp->t_inpcb->inp_options, &tp->t_inpcb->inp_route, >> - (so->so_options & SO_DONTROUTE), 0); >> + soopts, 0); >> } >> >> Locking and immediately unlocking accesses to a socket field doesn't >> accomplish anything. We want to lock the socket at the beginning of an >> operation and release it at the end. This leads to way fewer locks inserted >> into the networking code. jhb> Agreed, the value you read with the lock held above becomes invalid/stale as jhb> soon as you drop the lock, so the lock really isn't doing much good. It is, jhb> however, obfuscating the code and will probably have to be backed out when jhb> the code is more fully locked. Changes like this don't make reading/writing jhb> so_options safe. It is not enough just to lock the reads and writes, you jhb> need to lock the results of actions taken on those values as well to ensure jhb> that multiple actions taken on an object all perform them on a consistent jhb> snapshot of that object. When we call a function the network swi may call, it should be all right to lock a socket across the function because it does not block. ip_output(), tcp_output(), and udp_output() should be safe. Although I have not investigated completely yet, any functions called via pru_send of struct pr_usrreqs seem to be OK. >> Furthermore, these mechanical bottom-up socket locks don't respect >> lock ordering with respect to the top-down inpcb locks. Fortunately, >> cvs update only resulted in a few conflicts with the inpcb locking code >> and I can just turn off the socket lock macros in my tree. >> >> We should coordiante better on working to lock up the networking code so >> we're not working at cross-purposes. If we were to lock a PCB prior to a socket, we have to add a new method (pru_lock?) to struct struct pr_usrreqs so that the socket APIs can lock a PCB opaque to a socket. jhb> Can the various folks working on the network stack possibly "sit down" and jhb> write up a description of the overall locking strategy for the stack including jhb> the required lock orders, etc.? That would be a big help in coordinating jhb> things better. jhb> -- jhb> John Baldwin <jhb@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/ jhb> "Power Users Use the Power to Serve!" - http://www.FreeBSD.org/ jhb> To Unsubscribe: send mail to majordomo@FreeBSD.org jhb> with "unsubscribe freebsd-smp" in the body of the message -- Seigo Tanimura <tanimura@r.dl.itc.u-tokyo.ac.jp> <tanimura@FreeBSD.org> To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-smp" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200205240618.g4O6Ie3i020960>