Skip site navigation (1)Skip section navigation (2) 
Date:      Wed, 24 Jul 1996 11:51:11 -0700
From:      "Jin Guojun[ITG]" <jin@george.lbl.gov>
To:        mckusick@McKusick.COM
Cc:        bugs@freebsd.org
Subject:   m_freem lock up the kernel
Message-ID:  <199607241851.LAA17948@george.lbl.gov>
next in thread | raw e-mail | index | archive | help 

Dr. McKusick,

Should the kernel do a little sanity check? I took a couple of week to
find out a bug in someone's driver which causes system hang.
This may also reflect a system problem that wants to have a very clean device
driver code for the kernel.  However, I found many device drivers, which is made
in the spare time, are not that professional.  So, I wonder that some kernel
code, like free,  may need to be more friendly.

Here is the device driver:

	if (failed)	{
		message
		m_freem(mp);
		goto	bailout;
	}
	...

bailout:
	m_freem(mp);
	return	err_code;
}

----------------------------- kernel --------------------------

void
m_freem(m)
        register struct mbuf *m;
{
        register struct mbuf *n;

        if (m == NULL)
                return;
        do {
                MFREE(m, n);
                m = n;
        } while (m);
}

====== FreeBSD 2.2-SNAP MFREE ===== seems no where can be locked, but ??? ======
#ifdef notyet
#define MFREE(m, n) \ 
        { MBUFLOCK(mbstat.m_mtypes[(m)->m_type]--;) \
          if ((m)->m_flags & M_EXT) { \
                if ((m)->m_ext.ext_free) \
                        (*((m)->m_ext.ext_free))((m)->m_ext.ext_buf, \
                            (m)->m_ext.ext_size); \
                else { \
                        char *p = (m)->m_ext.ext_buf; \
                        if (--mclrefcnt[mtocl(p)] == 0) { \
                        ((union mcluster *)(p))->mcl_next = mclfree; \
                        mclfree = (union mcluster *)(p); \
                        mbstat.m_clfree++; \
                } \
          } \
          (n) = (m)->m_next; \
          (m)->m_type = MT_FREE; \
          mbstat.m_mtypes[MT_FREE]++; \
          (m)->m_next = mmbfree; \ 
          mmbfree = (m); \
        }
#else /* notyet */
#define MFREE(m, nn) \
        MBUFLOCK ( \
                mbstat.m_mtypes[(m)->m_type]--; \
                if ((m)->m_flags & M_EXT) { \
                        char *p = (m)->m_ext.ext_buf; \
                        if (--mclrefcnt[mtocl(p)] == 0) { \
                                ((union mcluster *)(p))->mcl_next = mclfree; \
                                mclfree = (union mcluster *)(p); \
                                mbstat.m_clfree++; \
                        } \
                } \
                (nn) = (m)->m_next; \
                (m)->m_type = MT_FREE; \
                mbstat.m_mtypes[MT_FREE]++; \
                (m)->m_next = mmbfree; \
                mmbfree = (m); \
        )
#endif
%%%%%%%%% This is different from the orginal 4.4 Lite-1/2 %%%%%%%%%%%%%

Both MFREE code in 4.4 Lite-1/2 and FreeBSD 2.2-SNAP does lock the kernel
if the same mbuf is freed by m_freem(mbuf) more than once.

Can we have a special m_type = INVALID_MBUF ?
then do this in MFREE(m, n)

#define MFREE(m, nn) \
	if ((m)->m_type == INVALID_MBUF)	{	\
		nn = NULL;	\
	} else	{	\
		MBUFLOCK(mbstat.m_mtypes[(m)->m_type]--;)	\
		...
		FREE(...)	\
		(m)->m_type = INVALID_MBUF;	\
	}

This can avoid the such code to lock the system.  The lock up the system
is much worse than crash / panic, which can reboot automatically.
If the system is locked, then someone has to be on site to reset or
power cycle the machine.

Would you like to have some suggestion?

	-Jin

P.S.	BTW, I would like to request a IORW() number for ATM ioctl,

	#define	SIOCATMCFG	_IOWR('i', 61, struct ifreq)

	The number "61" is a temporary number I am using.
	From whom should I ask for or register this number?

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199607241851.LAA17948>