Re: panic: ifc_free_unit: bit is already cleared

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 _at_ 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
<ifn>_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. :)

-- Brooks

-- 
Any statement of the form "X is the one, true Y" is FALSE.
PGP fingerprint 655D 519C 26A7 82E7 2529  9BF0 5D8E 8BE9 F238 1AD4