cvs commit: src/sys/sys mbuf.h src/sys/kern uipc_mbuf2.c
sam at errno.com
Sun Oct 10 15:24:12 PDT 2004
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> 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
> 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
> 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> 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 .
More information about the cvs-src