From owner-freebsd-current@FreeBSD.ORG Tue Oct 11 22:18:49 2005 Return-Path: X-Original-To: current@freebsd.org Delivered-To: freebsd-current@FreeBSD.ORG Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 9176A16A41F; Tue, 11 Oct 2005 22:18:49 +0000 (GMT) (envelope-from sam@errno.com) Received: from ebb.errno.com (69-12-149-25.dsl.static.sonic.net [69.12.149.25]) by mx1.FreeBSD.org (Postfix) with ESMTP id 0C23543D46; Tue, 11 Oct 2005 22:18:48 +0000 (GMT) (envelope-from sam@errno.com) Received: from [10.0.0.200] ([10.0.0.200]) (authenticated bits=0) by ebb.errno.com (8.12.9/8.12.6) with ESMTP id j9BMIVpU001462 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 11 Oct 2005 15:18:32 -0700 (PDT) (envelope-from sam@errno.com) Message-ID: <434C3A27.2000808@errno.com> Date: Tue, 11 Oct 2005 15:18:15 -0700 From: Sam Leffler User-Agent: Mozilla Thunderbird 1.0.6 (X11/20050927) X-Accept-Language: en-us, en MIME-Version: 1.0 To: Brooks Davis References: <20051005024903.GA72743@heff.fud.org.nz> <20051005203639.GA20552@garage.freebsd.pl> <20051005205515.GA30350@odin.ac.hmc.edu> <20051005210950.GB75848@heff.fud.org.nz> <20051009232849.GA27349@comp.chem.msu.su> <20051010022208.GA97249@heff.fud.org.nz> <20051010202900.GA24213@odin.ac.hmc.edu> <20051011210602.GA5714@heff.fud.org.nz> <20051011220749.GD13461@odin.ac.hmc.edu> In-Reply-To: <20051011220749.GD13461@odin.ac.hmc.edu> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Yar Tikhiy , Pawel Jakub Dawidek , FreeBSD Current , Andrew Thompson Subject: Re: panic: ifc_free_unit: bit is already cleared X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 11 Oct 2005 22:18:49 -0000 Brooks Davis wrote: > On Wed, Oct 12, 2005 at 10:06:02AM +1300, Andrew Thompson wrote: > >>On Mon, Oct 10, 2005 at 01:29:00PM -0700, Brooks Davis wrote: >> >>>On Mon, Oct 10, 2005 at 03:22:08PM +1300, Andrew Thompson wrote: >>> >>>>On Mon, Oct 10, 2005 at 03:28:49AM +0400, Yar Tikhiy wrote: >>>> >>>>>FWIW, I tried to look at the $subject problem since I had had it >>>>>before, but just got a different panic: >>>>> >>>>> Memory modified after free 0xc140b000(4092) val=deadc0dc @ 0xc140b000 >>>>> panic: Most recently used by clone >>>>> >>>>>The clone code seems to have decremented something (refcount?) twice >>>>>after freeing the memory chunk. >>>> >>>>I have been testing this patch and I think it fixes all the problems >>>>discussed. >>>> >>>>It changes refcounting to count the number of cloned interfaces so >>>>ifc_units is only freed when its safe. A new function has been added to >>>>decrement this when a simple cloner module is unloaded. The cloner is >>>>still detached first to prevent the race. >>>> >>>>In most cases the change is as simple as: >>> >>>I don't see any reason why you can't just replace the specific destroy >>>calls with calls to ifc_simple_destroy(). That would avoid expanding >>>the API. >> >>I have updated the patch and yes, its a nicer way to do it. Please >>review. >> >>Ive run through interations of create/kldunload with bridge, disc, >>faith, gif, gre and ppp with extra printf's and its freeing correctly. > > > This looks good to me, thanks for working on this and doing the > _destory removals. Let's see about getting this committed. > > Slightly longer term I think should consider hanging the interface list > off the cloner or maybe off some more generic per driver struct so > if_clone_detach() can destroy the interfaces and the unload code becomes > a one-liner in most cases. The current code is a result of not wanting > to mess with the drivers too much initially, but I think we need to > start looking at moving more bits into the support code. After all, if > we only write something once instead of once per interface, that's a lot > less opportunities to screw up. :) There's also the changes I have to pass parameter blocks on clone create... Sam