From owner-svn-src-head@FreeBSD.ORG Thu Oct 17 22:08:33 2013 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id 9C1FA2D0; Thu, 17 Oct 2013 22:08:33 +0000 (UTC) (envelope-from zec@fer.hr) Received: from mail.fer.hr (mail.fer.hr [161.53.72.233]) (using TLSv1 with cipher AES128-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id F2CB326AD; Thu, 17 Oct 2013 22:08:32 +0000 (UTC) Received: from x23.lan (89.164.204.125) by MAIL.fer.hr (161.53.72.233) with Microsoft SMTP Server (TLS) id 14.2.342.3; Fri, 18 Oct 2013 00:07:18 +0200 From: Marko Zec To: Subject: Re: svn commit: r228969 - head/sys/netinet Date: Fri, 18 Oct 2013 00:07:35 +0200 User-Agent: KMail/1.9.10 References: <201112292041.pBTKfGkj071711@svn.freebsd.org> <201310161709.04489.jhb@freebsd.org> <20131017193545.GA3683@gmail.com> In-Reply-To: <20131017193545.GA3683@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-ID: <201310180007.36048.zec@fer.hr> X-Originating-IP: [89.164.204.125] Cc: src-committers@freebsd.org, John Baldwin , svn-src-all@freebsd.org, hrs@freebsd.org, Mikolaj Golub , Bjoern Zeeb , svn-src-head@freebsd.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 17 Oct 2013 22:08:33 -0000 On Thursday 17 October 2013 21:35:46 Mikolaj Golub wrote: > On Wed, Oct 16, 2013 at 05:09:04PM -0400, John Baldwin wrote: > ... > > > > >> #10 0x80b6f1fd in igmp_change_state (inm=0x8ae70480) > > > >> at /home/golub/freebsd/base/head/sys/netinet/igmp.c:2595 > > > >> #11 0x80b76f68 in in_leavegroup_locked (inm=0x8ae70480, > > > >> imf=0x8a655a00) at > > > >> /home/golub/freebsd/base/head/sys/netinet/in_mcast.c:1239 #12 > > > >> 0x80b76fbd in in_leavegroup (inm=0x8ae70480, imf=0x8a655a00) at > > > >> /home/golub/freebsd/base/head/sys/netinet/in_mcast.c:1184 #13 > > > >> 0x80b770b4 in inp_gcmoptions (context=0x0, pending=1) at > > > >> /home/golub/freebsd/base/head/sys/netinet/in_mcast.c:1554 #14 > > > >> 0x80a8ff2b in taskqueue_run_locked (queue=0x87594880) at > > > >> /home/golub/freebsd/base/head/sys/kern/subr_taskqueue.c:308 #15 > > > >> 0x80a90987 in taskqueue_thread_loop (arg=0x81186bcc) at > > > >> /home/golub/freebsd/base/head/sys/kern/subr_taskqueue.c:497 #16 > > > >> 0x80a1b2d8 in fork_exit (callout=0x80a90920 > > > >> , arg=0x81186bcc, > > > >> frame=0x872b2d28) at > > > >> /home/golub/freebsd/base/head/sys/kern/kern_fork.c:992 > > ... > > > > >> VNET context is not set at that point. > > ... > > > I think this was just fixed by glebius@ in r256587: > > > > Author: glebius > > Date: Wed Oct 16 05:02:01 2013 > > New Revision: 256587 > > URL: http://svnweb.freebsd.org/changeset/base/256587 > > > > Log: > > For VIMAGE kernels store vnet in the struct task, and set vnet > > context during task processing. > > > > Reported & tested by: mm > > I think that particular issue was fixed earlier by hrs in r252510: > > Fix a panic when leaving MC group in a kernel with VIMAGE enabled. > in_leavegroup() is called from an asynchronous task, and > igmp_change_state() requires that curvnet is set by the caller. > > Concerning this more general solution from Gleb, with storing the vnet > pointer in the task, I wander how it is safe when the vnet is being > removed? > > In vnet_destroy() the vnet interfaces are moved back to their parent > vnets, so setting the vnet context from the interface looks safer. +1 Gleb, what exactly was the justification for embedding a vnet context inside struct task, when it could have been easily embedded, directly or indirectly, inside the object pointed to by ta_context? What is / was your plan to garbage collect such stale pointers? At least in the early days of VIMAGE / VNET hacking, all the people involved strived very hard to store backpointers to vnets in as few structs as possible, and for quite a while we managed to constrain this to only ifnets and sockets, and both of those references are refcounted in struct vnet. Now, more (unreferenced) backpointers to struct vnet seem to be popping up here and there, but in most cases they are embedded in objects which are guaranteed to persist only as long as the referenced vnet is alive. One notable exception to this rule is struct tcpcb which apparently can outlive vnet teardowns under certain conditions, but this is a known issue which needs to be fixed, not a precedent which calls for more timebombs to be scattered around the source tree. So pls. reconsider solving the issue at hand using a different approach, and back out r256587. Thanks, Marko