Re: MBUF locking- this can't be right, can it?
> On Wed, 13 Jun 2001 14:15:53 -0400
> Bosko Milekic <[EMAIL PROTECTED]> said:
> 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> Nope, no objection here. :-)
I just committed. Thanks!!
--
Hajimu UMEMOTO @ Internet Mutual Aid Society Yokohama, Japan
[EMAIL PROTECTED] [EMAIL PROTECTED] ume@{,jp.}FreeBSD.org
http://www.imasy.org/~ume/
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
Re: MBUF locking- this can't be right, can it?
On Thu, Jun 14, 2001 at 03:12:06AM +0900, Hajimu UMEMOTO wrote: > > On Wed, 13 Jun 2001 13:42:51 -0400 > > Bosko Milekic <[EMAIL PROTECTED]> 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. Nope, no objection here. :-) Regards, -- Bosko Milekic [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: MBUF locking- this can't be right, can it?
On Thu, 14 Jun 2001, Hajimu UMEMOTO wrote: > > On Wed, 13 Jun 2001 13:42:51 -0400 > > Bosko Milekic <[EMAIL PROTECTED]> 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. > Works for me. I haven't completed building a new kernel and testing though. -matt To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: MBUF locking- this can't be right, can it?
> On Wed, 13 Jun 2001 13:42:51 -0400
> Bosko Milekic <[EMAIL PROTECTED]> 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 [EMAIL PROTECTED]
> with "unsubscribe freebsd-current" in the body of the message
>
bmilekic> --
bmilekic> Bosko Milekic
bmilekic> [EMAIL PROTECTED]
bmilekic> To Unsubscribe: send mail to [EMAIL PROTECTED]
bmilekic> with "unsubscribe freebsd-current" in the body of the message
To Unsubsc
Re: MBUF locking- this can't be right, can it?
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 [EMAIL PROTECTED]
> with "unsubscribe freebsd-current" in the body of the message
>
--
Bosko Milekic
[EMAIL PROTECTED]
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
MBUF locking- this can't be right, can it?
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 [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
