From owner-freebsd-current Wed Jun 13 11:12:32 2001 Delivered-To: freebsd-current@freebsd.org Received: from peace.mahoroba.org (peace.calm.imasy.or.jp [202.227.26.34]) by hub.freebsd.org (Postfix) with ESMTP id 9D28F37B401 for ; Wed, 13 Jun 2001 11:12:21 -0700 (PDT) (envelope-from ume@mahoroba.org) Received: from localhost (IDENT:ZUzhLGfZktqOpPIACh9EJHb2bCMz/WVAPP0kYTMX4dA1n6bBuFNR5dRuUgDrnxcX@localhost [::1]) (authenticated as ume with CRAM-MD5) by peace.mahoroba.org (8.11.4/8.11.4/peace) with ESMTP/inet6 id f5DIC9q31515; Thu, 14 Jun 2001 03:12:09 +0900 (JST) (envelope-from ume@mahoroba.org) Date: Thu, 14 Jun 2001 03:12:06 +0900 (JST) Message-Id: <20010614.031206.85718093.ume@mahoroba.org> To: bmilekic@technokratis.com Cc: mjacob@feral.com, freebsd-current@FreeBSD.ORG Subject: Re: MBUF locking- this can't be right, can it? From: Hajimu UMEMOTO In-Reply-To: <20010613134251.A8764@technokratis.com> References: <20010613134251.A8764@technokratis.com> X-Mailer: xcite1.38> Mew version 1.95b119 on Emacs 20.7 / Mule 4.0 =?iso-2022-jp?B?KBskQjJWMWMbKEIp?= X-PGP-Public-Key: http://www.imasy.org/~ume/publickey.asc X-PGP-Fingerprint: 6B 0C 53 FC 5D D0 37 91 05 D0 B3 EF 36 9B 6A BC X-URL: http://www.imasy.org/~ume/ X-Operating-System: FreeBSD 5.0-CURRENT Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Sender: owner-freebsd-current@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG >>>>> On Wed, 13 Jun 2001 13:42:51 -0400 >>>>> Bosko Milekic said: bmilekic> Yes, I certainly didn't write that. I think it was KAME. Yup, current KAME is based on 4.3-RELEASE which doesn't have mtx_lock() issue. It is my mistake during merging it into 5-CURRENT. The fix looks good to me. If there is no objection, I'll commit it. bmilekic> 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 > bmilekic> -- bmilekic> Bosko Milekic bmilekic> bmilekic@technokratis.com bmilekic> To Unsubscribe: send mail to majordomo@FreeBSD.org bmilekic> with "unsubscribe freebsd-current" in the body of the message To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message