Bosko Milekic writes:
 > 
 > On Thu, Jan 02, 2003 at 01:53:53PM -0500, Andrew Gallatin wrote:
 > > I'm just tuning up my driver now to catch up to the "recent" interface
 > > changes.  While there, I went to add a ref count for my driver managed
 > > M_EXT clusters.  However, m_extadd() does not take a parameter for
 > > assignment into mp->m_ext.ref_cnt Eg, I cannot call m_extadd() if I
 > > want to use my own refcounter.
 > > 
 > > Is there any chance this could be fixed?  O/w, I'll have to
 > > avoid calling m_extadd()..
 > 
 >   I dunno.  I hate the whole story behind the reference counters in the
 >   mbuf code and I don't know what the correct approach here would be.  A
 >   long long while back I think m_extadd (or its equivalent) did allow
 >   something like this.  Then, we changed it so that counters would be
 >   allocated by the mbuf code 'transparently' (i.e., so that the Rest Of
 >   The World didn't have to worry about it).  However, not long ago, I
 >   changed the way reference counters were allocated for mbuf clusters so
 >   that it would be faster.  Counters for other objects are allocated
 >   with malloc().  The only 'correct' (half-assed) solution I can see is
 >   to allow m_extadd() to take a 'refcount' argument (again?) - perhaps
 >   leave a backwards-compatible m_extadd() and add a m_addext() or
 >   something - and only call malloc() for the counter if refcnt_provided
 >   == NULL.
 > 
 >   To be honest, I don't really like the idea but I don't see a better
 >   solution.  Right now, ref counting for regular mbuf clusters works
 >   fine and is pretty damn fast, but I don't know how I could make it
 >   happen for other external buffer types other than the way I just
 >   described.

There's a second can of worms too.  We don't want the mbuf system
freeing externally maintained refcnt pointers.  So we need a type
or flag to distinguish them.

I've appended a minimal impact patch that I came up with.  It just
adds a new type (EXT_EXTREF) and leaves the m_extadd() api/abi pretty
much unchanged.  Callers that want to manage their own refcnt memory
call m_extadd() like this:

      mp->m_ext.ref_cnt =  &entry->ref_count;
      MEXTADD(mp, (void *)entry->jbuf->buf, GM_JLEN,
                    gm_freebsd_ether_jfree, (void *)entry->jbuf, 
                    0, EXT_EXTREF);


It seems to work just fine, and together with some patches from Alan
Cox for kmem_malloc(), allows me to make my network driver MPSAFE.
I'm still testing for other hidden Giant acquisitions or
GIANT_REQUIRED() assertions in rare codepaths, but its been up for 15
minutes now, and that's 14m 59sec  longer than usual ;)

Would you be OK with this or something like it?

Thanks,

Drew


Index: kern/subr_mbuf.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/subr_mbuf.c,v
retrieving revision 1.34
diff -u -r1.34 subr_mbuf.c
--- kern/subr_mbuf.c    27 Nov 2002 04:26:00 -0000      1.34
+++ kern/subr_mbuf.c    2 Jan 2003 19:59:54 -0000
@@ -1062,7 +1062,8 @@
     (((uintptr_t)(cl) - (uintptr_t)cl_refcntmap) >> MCLSHIFT)
 
 #define        _mext_dealloc_ref(m)                                            \
-       free((m)->m_ext.ref_cnt, M_MBUF)
+       if ((m)->m_ext.ext_type != EXT_EXTREF)                          \
+               free((m)->m_ext.ref_cnt, M_MBUF)
 
 /******************************************************************************
  * Internal routines.
@@ -1508,9 +1509,13 @@
 m_extadd(struct mbuf *mb, caddr_t buf, u_int size,
     void (*freef)(void *, void *), void *args, int flags, int type)
 {
+       u_int *ref_cnt = NULL;
 
-       _mext_init_ref(mb, ((type != EXT_CLUSTER) ?
-           NULL : &cl_refcntmap[cl2ref(mb->m_ext.ext_buf)]));
+       if (type == EXT_CLUSTER)
+               ref_cnt = &cl_refcntmap[cl2ref(mb->m_ext.ext_buf)];
+       else if (type == EXT_EXTREF)
+               ref_cnt = mb->m_ext.ref_cnt;
+       _mext_init_ref(mb, ref_cnt);
        if (mb->m_ext.ref_cnt != NULL) {
                mb->m_flags |= (M_EXT | flags);
                mb->m_ext.ext_buf = buf;
Index: sys/mbuf.h
===================================================================
RCS file: /home/ncvs/src/sys/sys/mbuf.h,v
retrieving revision 1.109
diff -u -r1.109 mbuf.h
--- sys/mbuf.h  30 Dec 2002 20:22:40 -0000      1.109
+++ sys/mbuf.h  2 Jan 2003 19:40:20 -0000
@@ -173,6 +173,7 @@
 #define        EXT_NET_DRV     100     /* custom ext_buf provided by net driver(s) */
 #define        EXT_MOD_TYPE    200     /* custom module's ext_buf type */
 #define        EXT_DISPOSABLE  300     /* can throw this buffer away w/page flipping 
*/
+#define        EXT_EXTREF      400     /* has externally maintained ref_cnt ptr*/
 
 /*
  * Flags copied when copying m_pkthdr.

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message

Reply via email to