From owner-freebsd-current  Wed Jun 13 10:42:29 2001
Delivered-To: freebsd-current@freebsd.org
Received: from technokratis.com (modemcable052.174-202-24.mtl.mc.videotron.ca [24.202.174.52])
	by hub.freebsd.org (Postfix) with ESMTP id EC30B37B403
	for <freebsd-current@FreeBSD.ORG>; Wed, 13 Jun 2001 10:42:10 -0700 (PDT)
	(envelope-from bmilekic@technokratis.com)
Received: (from bmilekic@localhost)
	by technokratis.com (8.11.3/8.11.3) id f5DHgpF08782;
	Wed, 13 Jun 2001 13:42:51 -0400 (EDT)
	(envelope-from bmilekic)
Date: Wed, 13 Jun 2001 13:42:51 -0400
From: Bosko Milekic <bmilekic@technokratis.com>
To: Matthew Jacob <mjacob@feral.com>
Cc: freebsd-current@FreeBSD.ORG
Subject: Re: MBUF locking- this can't be right, can it?
Message-ID: <20010613134251.A8764@technokratis.com>
References: <Pine.BSF.4.21.0106131033130.40934-100000@beppo.feral.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.2.5i
In-Reply-To: <Pine.BSF.4.21.0106131033130.40934-100000@beppo.feral.com>; from mjacob@feral.com on Wed, Jun 13, 2001 at 10:37:22AM -0700
Sender: owner-freebsd-current@FreeBSD.ORG
Precedence: bulk
List-ID: <freebsd-current.FreeBSD.ORG>
List-Archive: <http://docs.freebsd.org/mail/> (Web Archive)
List-Help: <mailto:majordomo?subject=help> (List Instructions)
List-Subscribe: <mailto:majordomo?subject=subscribe%20freebsd-current>
List-Unsubscribe: <mailto:majordomo?subject=unsubscribe%20freebsd-current>
X-Loop: FreeBSD.ORG


	Yes, I certainly didn't write that. I think it was KAME.

On Wed, Jun 13, 2001 at 10:37:22AM -0700, Matthew Jacob wrote:
> 
> A recent change to the MFREE macro was made as noted below:
> 
> /*
>  * MFREE(struct mbuf *m, struct mbuf *n)
>  * Free a single mbuf and associated external storage.
>  * Place the successor, if any, in n.
>  * 
>  * we do need to check non-first mbuf for m_aux, since some of existing
>  * code does not call M_PREPEND properly.
>  * (example: call to bpf_mtap from drivers)
>  */
> #define MFREE(m, n) do {                                                \
>         struct mbuf *_mm = (m);                                         \
>                                                                         \
>         KASSERT(_mm->m_type != MT_FREE, ("freeing free mbuf"));         \
>         if (_mm->m_flags & M_EXT)                                       \
>                 MEXTFREE(_mm);                                          \
>         mtx_lock(&mbuf_mtx);                                            \
>         mbtypes[_mm->m_type]--;                                         \
> ++        if ((_mm->m_flags & M_PKTHDR) != 0 && _mm->m_pkthdr.aux) {      \
> ++                m_freem(_mm->m_pkthdr.aux);                             \
> ++                _mm->m_pkthdr.aux = NULL;                               \
> ++        }                                                               \
>         _mm->m_type = MT_FREE;                                          \
>         mbtypes[MT_FREE]++;                                             \
>         (n) = _mm->m_next;                                              \
>         _mm->m_next = mmbfree.m_head;                                   \
>         mmbfree.m_head = _mm;                                           \
>         MBWAKEUP(m_mballoc_wid, &mmbfree.m_starved);                    \
>         mtx_unlock(&mbuf_mtx);                                          \
> } while (0)
> 
> 
> Unfortunately, m_freem also calls MFREE. Tsk. Now, it seems to me that even in
> cases where you *could* allow recursive locks, the right idea here would be to
> change it to:
> 
> /*
>  * MFREE(struct mbuf *m, struct mbuf *n)
>  * Free a single mbuf and associated external storage.
>  * Place the successor, if any, in n.
>  * 
>  * we do need to check non-first mbuf for m_aux, since some of existing
>  * code does not call M_PREPEND properly.
>  * (example: call to bpf_mtap from drivers)
>  */
> #define MFREE(m, n) do {                                                \
>         struct mbuf *_mm = (m);                                         \
> 	struct mbuf *_aux;                                              \
>                                                                         \
>         KASSERT(_mm->m_type != MT_FREE, ("freeing free mbuf"));         \
>         if (_mm->m_flags & M_EXT)                                       \
>                 MEXTFREE(_mm);                                          \
>         mtx_lock(&mbuf_mtx);                                            \
>         mbtypes[_mm->m_type]--;                                         \
>         if ((_mm->m_flags & M_PKTHDR) != 0 && _mm->m_pkthdr.aux) {      \
> 		_aux = _mm->m_pkthdr.aux;                               \
>                 _mm->m_pkthdr.aux = NULL;                               \
>         } else {                                                        \
>                 _aux = NULL;                                            \
>         }                                                               \
>         _mm->m_type = MT_FREE;                                          \
>         mbtypes[MT_FREE]++;                                             \
>         (n) = _mm->m_next;                                              \
>         _mm->m_next = mmbfree.m_head;                                   \
>         mmbfree.m_head = _mm;                                           \
>         MBWAKEUP(m_mballoc_wid, &mmbfree.m_starved);                    \
>         mtx_unlock(&mbuf_mtx);                                          \
>         if (_aux)                                                       \
>                 m_freem(_aux);                                          \
> } while (0)
> 
> 
> Comments?
> 
> -matt
> 
> 
> 
> To Unsubscribe: send mail to majordomo@FreeBSD.org
> with "unsubscribe freebsd-current" in the body of the message
> 

-- 
 Bosko Milekic
 bmilekic@technokratis.com


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message