Date: Thu, 7 Oct 2004 15:11:48 -0400 From: Vlad <marchenko@gmail.com> To: Brian Fundakowski Feldman <green@freebsd.org> Cc: Marc UBM Bocklet <ubm@u-boot-man.de> Subject: Re: [BETA7-panic] sodealloc(): so_count 1 Message-ID: <cd70c681041007121113048277@mail.gmail.com> In-Reply-To: <20041007181852.GA73261@green.homeunix.org> References: <20041006015131.10116be7.ubm@u-boot-man.de> <cd70c68104100517074a5cebf2@mail.gmail.com> <20041006090104.06710d85.ubm@u-boot-man.de> <20041006154137.GJ47017@green.homeunix.org> <20041006203220.7f8e7b8a.ubm@u-boot-man.de> <20041006192518.GM47017@green.homeunix.org> <cd70c681041006125527e69bcd@mail.gmail.com> <20041006215134.GN47017@green.homeunix.org> <20041007010008.15274fbd.ubm@u-boot-man.de> <20041007181852.GA73261@green.homeunix.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Brian, thanks for your work. Question: is this patch is something I can feel comfortable about trying it on a production server? Did you test it? On Thu, 7 Oct 2004 14:18:52 -0400, Brian Fundakowski Feldman <green@freebsd.org> wrote: > > > On Thu, Oct 07, 2004 at 01:00:08AM +0200, Marc UBM Bocklet wrote: > > On Wed, 6 Oct 2004 17:51:34 -0400 > > Brian Fundakowski Feldman <green@freebsd.org> wrote: > > > > > On Wed, Oct 06, 2004 at 03:55:21PM -0400, Vlad wrote: > > > > Brian, > > > > > > > > I've created ticket a while ago in regards to this problem: > > > > > > > > http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/72126 > > > > > > > > also, attached are some additional debug info I could get during > > > > last two times it crashed. > > > > > > > > unfortunately I don't have big enough stand-alone swap partition to > > > > get kernel crash dump, so those files are the best I could get. > > > > > > > > here is week-old thread for the same problem: > > > > http://groups.google.com/groups?th=fc0d7d881f0713cc > > > > > > Can you tell me where sorele() crashed when you did _not_ have > > > INVARIANTS enabled? That will help because the INVARIANTS panic tells > > > us one half of where the problem is and the sorele() panic is the > > > other half. Want to try this (untested) patch for the problem that I'm > > > guessing about? > > > > [possible patch for sodealloc panic] > > > > Ok, I'm just now recompiling my kernel with your patch, I should be able > > to boot the kernel tomorrow, then we'll see if there are any further > > panics. > > > > Thanks a lot for your help! :-) > > You're welcome! If that does work (or mostly works), please try these > deltas instead. I have only tried to make TCP work correctly in this > respect, but all protocols need to be able to atomically with regard > to both the accept lock and the socket lock give up their pcb from > the proto bottom half and give final "ownership" of the socket to its > parent (listening socket) before never touching it again. > > The order of SOCK_LOCK(), so_pcb = NULL, check for 0 references, check > for no fd reference, SOCK_UNLOCK(), ACCEPT_LOCK() violates this. This > is mostly something that could easily be fixed by using regular GC > methods to do the final bit of deallocation. > > Index: sys/socketvar.h > =================================================================== > RCS file: /usr/ncvs/src/sys/sys/socketvar.h,v > retrieving revision 1.133 > diff -u -r1.133 socketvar.h > --- sys/socketvar.h 12 Jul 2004 21:42:33 -0000 1.133 > +++ sys/socketvar.h 7 Oct 2004 17:52:49 -0000 > @@ -205,6 +205,7 @@ > #define SS_ASYNC 0x0200 /* async i/o notify */ > #define SS_ISCONFIRMING 0x0400 /* deciding to accept connection req */ > #define SS_ISDISCONNECTED 0x2000 /* socket disconnected from peer */ > +#define SS_HASBEENCOMP 0x4000 /* proto made socket SQ_COMP */ > > /* > * Socket state bits now stored in the socket buffer state field. > @@ -360,6 +361,16 @@ > SOCK_UNLOCK(so); \ > } while(0) > > +#define sopcbfreed(so) do { \ > + SOCK_LOCK_ASSERT(so); \ > + (so)->so_pcb = NULL; \ > + if ((so)->so_count == 0 && \ > + ((so)->so_state & SS_HASBEENCOMP) == 0) \ > + sofree(so); \ > + else \ > + SOCK_UNLOCK(so); \ > +} while(0) > + > /* > * In sorwakeup() and sowwakeup(), acquire the socket buffer lock to > * avoid a non-atomic test-and-wakeup. However, sowakeup is > Index: kern/uipc_socket2.c > =================================================================== > RCS file: /usr/ncvs/src/sys/kern/uipc_socket2.c,v > retrieving revision 1.137 > diff -u -r1.137 uipc_socket2.c > --- kern/uipc_socket2.c 15 Aug 2004 06:24:41 -0000 1.137 > +++ kern/uipc_socket2.c 7 Oct 2004 17:42:15 -0000 > @@ -117,12 +117,15 @@ > { > struct socket *head; > > + ACCEPT_LOCK(); > SOCK_LOCK(so); > so->so_state &= ~(SS_ISCONNECTING|SS_ISDISCONNECTING|SS_ISCONFIRMING); > so->so_state |= SS_ISCONNECTED; > - SOCK_UNLOCK(so); > - ACCEPT_LOCK(); > head = so->so_head; > + if (head && (so->so_qstate & SQ_INCOMP) && > + (so->so_options & SO_ACCEPTFILTER) == 0) > + so->so_state |= SS_HASBEENCOMP; > + SOCK_UNLOCK(so); > if (head != NULL && (so->so_qstate & SQ_INCOMP)) { > if ((so->so_options & SO_ACCEPTFILTER) == 0) { > TAILQ_REMOVE(&head->so_incomp, so, so_list); > Index: kern/uipc_syscalls.c > =================================================================== > RCS file: /usr/ncvs/src/sys/kern/uipc_syscalls.c,v > retrieving revision 1.200 > diff -u -r1.200 uipc_syscalls.c > --- kern/uipc_syscalls.c 15 Aug 2004 06:24:41 -0000 1.200 > +++ kern/uipc_syscalls.c 7 Oct 2004 17:55:22 -0000 > @@ -314,9 +314,8 @@ > KASSERT(so->so_qstate & SQ_COMP, ("accept1: so not SQ_COMP")); > > /* > - * Before changing the flags on the socket, we have to bump the > - * reference count. Otherwise, if the protocol calls sofree(), > - * the socket will be released due to a zero refcount. > + * XXX This reference should really be done by soaccept(). Once > + * SQ_COMP, a socket will never disappear from underneath us. > */ > SOCK_LOCK(so); > soref(so); /* file descriptor reference */ > cvs diff: Diffing netinet > Index: netinet/in_pcb.c > =================================================================== > RCS file: /usr/ncvs/src/sys/netinet/in_pcb.c,v > retrieving revision 1.155 > diff -u -r1.155 in_pcb.c > --- netinet/in_pcb.c 29 Sep 2004 04:01:13 -0000 1.155 > +++ netinet/in_pcb.c 7 Oct 2004 17:58:38 -0000 > @@ -688,8 +688,7 @@ > in_pcbremlists(inp); > if (so) { > SOCK_LOCK(so); > - so->so_pcb = NULL; > - sotryfree(so); > + sopcbfreed(so); > } > if (inp->inp_options) > (void)m_free(inp->inp_options); > Index: netinet/tcp_subr.c > =================================================================== > RCS file: /usr/ncvs/src/sys/netinet/tcp_subr.c,v > retrieving revision 1.203 > diff -u -r1.203 tcp_subr.c > --- netinet/tcp_subr.c 5 Sep 2004 02:34:12 -0000 1.203 > +++ netinet/tcp_subr.c 7 Oct 2004 17:59:01 -0000 > @@ -1686,10 +1686,9 @@ > tcp_discardcb(tp); > so = inp->inp_socket; > SOCK_LOCK(so); > - so->so_pcb = NULL; > tw->tw_cred = crhold(so->so_cred); > tw->tw_so_options = so->so_options; > - sotryfree(so); > + sopcbfreed(so); > inp->inp_socket = NULL; > if (acknow) > tcp_twrespond(tw, TH_ACK); > > > > -- > Brian Fundakowski Feldman \'[ FreeBSD ]''''''''''\ > <> green@FreeBSD.org \ The Power to Serve! \ > Opinions expressed are my own. \,,,,,,,,,,,,,,,,,,,,,,\ > _______________________________________________ > freebsd-current@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-current > To unsubscribe, send any mail to "freebsd-current-unsubscribe@freebsd.org" > -- Vlad
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?cd70c681041007121113048277>