cvs commit: src/sys/sys mbuf.h src/sys/kern uipc_mbuf2.c src/share/man/man9 mbuf_tags.9

[Sam Leffler]

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