Date: Tue, 29 Nov 2011 13:35:50 +0400 From: Gleb Smirnoff <glebius@FreeBSD.org> To: "Bjoern A. Zeeb" <bz@FreeBSD.org> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r228071 - head/sys/net Message-ID: <20111129093550.GZ44498@FreeBSD.org> In-Reply-To: <E15FE643-3360-4D42-8736-827104FDD128@FreeBSD.org> References: <201111281444.pASEixdO095604@svn.freebsd.org> <E15FE643-3360-4D42-8736-827104FDD128@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Nov 28, 2011 at 07:47:22PM +0000, Bjoern A. Zeeb wrote: B> On 28. Nov 2011, at 14:44 , Gleb Smirnoff wrote: B> B> > Author: glebius B> > Date: Mon Nov 28 14:44:59 2011 B> > New Revision: 228071 B> > URL: http://svn.freebsd.org/changeset/base/228071 B> > B> > Log: B> > - Use generic alloc_unr(9) allocator for if_clone, instead B> > of hand-made. B> > - When registering new cloner, check whether a cloner with B> > same name already exist. B> > - When allocating unit, also check with help of ifunit() B> > whether such interface already exist or not. [1] B> B> This forces packages to be recompiled; they might like to have a __FreeBSD_version for that? B> It's not MFCable, at least I think - don't see a MFC after, just want to be sure. No plans for MFC. btw, I don't like the static initializer of cloners, since it require re-compile of dependencies. What about making an API change: remove the static initializer and make an initializer function. Modules should have only a pointer to opaque structure. We will bump __FreeBSD_version for that. But later with any change to the if_clone struct, we won't have ABI change. If everyone agrees, I can go for that. B> See one more comment inline? B> > + ifc->ifc_unrhdr = new_unrhdr(0, ifc->ifc_maxunit, &ifc->ifc_mtx); B> > + LIST_INIT(&ifc->ifc_iflist); B> > B> > IF_CLONERS_LOCK(); B> > + LIST_FOREACH(ifc1, &V_if_cloners, ifc_list) B> > + if (strcmp(ifc->ifc_name, ifc1->ifc_name) == 0) { B> > + IF_CLONERS_UNLOCK(); B> > + IF_CLONE_REMREF(ifc); B> > + return (EEXIST); B> B> At this point you may have a problem not freeing the unr? No, there is no problem. The code path that goes to delete_unrhdr() is the following: IF_CLONE_REMREF calls IF_CLONE_REMREF_LOCKED, the latter calls if_clone_free(), and the last one calls delete_unrhdr(). -- Totus tuus, Glebius.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20111129093550.GZ44498>