From owner-cvs-all@FreeBSD.ORG Sun Oct 10 22:24:11 2004 Return-Path: Delivered-To: cvs-all@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id A461A16A4CE; Sun, 10 Oct 2004 22:24:11 +0000 (GMT) Received: from ebb.errno.com (ebb.errno.com [66.127.85.87]) by mx1.FreeBSD.org (Postfix) with ESMTP id 5D0D943D4C; Sun, 10 Oct 2004 22:24:11 +0000 (GMT) (envelope-from sam@errno.com) Received: from [66.127.85.93] ([66.127.85.93]) (authenticated bits=0) by ebb.errno.com (8.12.9/8.12.6) with ESMTP id i9AMOAWi088717 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sun, 10 Oct 2004 15:24:11 -0700 (PDT) (envelope-from sam@errno.com) Message-ID: <4169B69F.8080708@errno.com> Date: Sun, 10 Oct 2004 15:24:31 -0700 From: Sam Leffler Organization: Errrno Consulting User-Agent: Mozilla Thunderbird 0.8 (Macintosh/20040913) X-Accept-Language: en-us, en MIME-Version: 1.0 To: Gleb Smirnoff References: <200410091325.i99DPK00097724@repoman.freebsd.org> <4168009A.303@errno.com> <20041009213710.GB8922@cell.sick.ru> <41686584.6070606@errno.com> <20041010093157.GA11523@cell.sick.ru> <4169AB03.7060105@errno.com> <20041010220921.GB14849@cell.sick.ru> In-Reply-To: <20041010220921.GB14849@cell.sick.ru> Content-Type: text/plain; charset=KOI8-R; format=flowed Content-Transfer-Encoding: 7bit cc: cvs-src@freebsd.org cc: src-committers@freebsd.org cc: cvs-all@freebsd.org Subject: Re: cvs commit: src/sys/sys mbuf.h src/sys/kern uipc_mbuf2.c src/share/man/man9 mbuf_tags.9 X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 10 Oct 2004 22:24:11 -0000 Gleb Smirnoff wrote: > On Sun, Oct 10, 2004 at 02:34:59PM -0700, Sam Leffler wrote: > S> >S> You did not find existing uses of subclassing because I backed out the > S> >S> vlan changes to use a private pool for unrelated reasons. I very very > S> >S> strongly disagree with this change and want it reverted. > S> > > S> >This was not broken. Look in my changes to uipc_mbuf2.c, you'll see that > S> >m_tag_delete() was changed so that it calls free thru method pointer. > S> >As I said, I've checked all m_tag_free() consumers, and nothing is > S> >affected. > S> > S> I'm sorry but one of us does not understand the issues. You changed > S> things so that calls to m_tag_free no longer invoked the free method. > S> This meant that every such call would do the wrong thing for tags > S> allocated using a private strategy. > > Yes, that's right. But m_tag_free is a public interface and changing what it does breaks existing code. > > S> Because you didn't see any of these > S> in the tree does not matter. Your change made it impossible to use > S> private allocation strategies. > > No, see below ... > > S> In fact with your change there was no > S> longer a reason to have a free method in the tag structure. > > My change does not affect potential private allocators. For example, it > does not conflict with if_vlan.c rev. 1.56, which I believe, you use as > an example. In if_vlan.c rev. 1.56 you use a private method for > free: > > mtag->m_tag_free = vlan_tag_free; > > When mbuf is m_freem()d, m_tag_delete() is called from mb_dtor_mbuf(). > Notice, that I have changed m_tag_delete() so that it calls private > method, and thus vlan_tag_free() will be called. > > So, I can't understand why do you call it broken. You are assuming tags resources are only reclaimed with m_tag_delete which is not correct. As I said above m_tag_free is a public interface and you've changed how it works (thus breaking compatibility with openbsd). If there were a good reason to change it then I'd be open to it, but all this got started simply because you wanted access to _m_tag_free and these other changes were uneeded. > > S> >My main purpose for this change was to create a possibility to create a > S> >custom > S> >free method, which inherits default method. How it is possible to do it > S> >now, without API change? > S> > S> You need to expose the previous _m_tag_free routine so it can be called. > S> My only request to you when you did this was to remove the leading '_' > S> as the routine was no longer going to be private to the file. What > S> seems to have confused you is that you not only need to remove the '_' > S> but also choose a different name so that it does not conflict with > S> m_tag_free defined in mbuf.h. I thought that was obvious but perhaps it > S> was not. > > OK, if we just rename it to something else, then both approaches will be > usable. What do you prefer m_tag_free_default() or m_tag_free1() or > smth else? I don't particularly care; probably m_tag_free_default . Sam