Use only one mutex for the entire mbuf subsystem.

Don't use atomic operations for the stats updating, instead protect
the counts with the mbuf mutex.  Most twiddling of the stats was
done right before or after releasing a mutex.  By doing this we
reduce the number of locked ops needed as well as allow a sysctl
to gain a consitant view of the entire stats structure.

In the future...

  This will allow us to chain common mbuf operations that would
  normally need to aquire/release 2 or 3 of the locks to build an
  mbuf with a cluster or external data attached into a single op
  requiring only one lock.

  Simplify the per-cpu locks that are planned.

There's also some if (1) code that should check if the "how"
operation specifies blocking/non-blocking behavior, we _could_ make
it so that we hold onto the mutex through calls into kmem_alloc
when non-blocking requests are made, but for safety reasons we
currently drop and reaquire the mutex around the calls.

Also, note that calling kmem_alloc is rare and only happens during
a shortage so drop/re-getting the mutex will not be a common
occurance.

Remove some #define's that seemed to obfuscate the code to me.

Remove an extranious comment.

Remove an XXX, including mutex.h isn't a crime.

Reviewed by: bmilekic
This commit is contained in:
Alfred Perlstein 2001-04-03 03:15:11 +00:00
parent 5b3047d59f
commit 5ea487f34d
2 changed files with 75 additions and 70 deletions

View File

@ -71,6 +71,7 @@ u_long m_clalloc_wid = 0;
struct mbffree_lst mmbfree;
struct mclfree_lst mclfree;
struct mcntfree_lst mcntfree;
struct mtx mbuf_mtx;
/*
* sysctl(8) exported objects
@ -135,9 +136,7 @@ mbinit(void *dummy)
mmbfree.m_head = NULL;
mclfree.m_head = NULL;
mcntfree.m_head = NULL;
mtx_init(&mmbfree.m_mtx, "mbuf free list lock", MTX_DEF);
mtx_init(&mclfree.m_mtx, "mcluster free list lock", MTX_DEF);
mtx_init(&mcntfree.m_mtx, "m_ext counter free list lock", MTX_DEF);
mtx_init(&mbuf_mtx, "mbuf free list lock", MTX_DEF);
/*
* Initialize mbuf subsystem (sysctl exported) statistics structure.
@ -151,20 +150,14 @@ mbinit(void *dummy)
/*
* Perform some initial allocations.
*/
mtx_lock(&mcntfree.m_mtx);
mtx_lock(&mbuf_mtx);
if (m_alloc_ref(REF_INIT, M_DONTWAIT) == 0)
goto bad;
mtx_unlock(&mcntfree.m_mtx);
mtx_lock(&mmbfree.m_mtx);
if (m_mballoc(NMB_INIT, M_DONTWAIT) == 0)
goto bad;
mtx_unlock(&mmbfree.m_mtx);
mtx_lock(&mclfree.m_mtx);
if (m_clalloc(NCL_INIT, M_DONTWAIT) == 0)
goto bad;
mtx_unlock(&mclfree.m_mtx);
mtx_unlock(&mbuf_mtx);
return;
bad:
@ -201,10 +194,12 @@ m_alloc_ref(u_int nmb, int how)
*/
nbytes = round_page(nmb * sizeof(union mext_refcnt));
mtx_unlock(&mcntfree.m_mtx);
if (1 /* XXX: how == M_TRYWAIT */)
mtx_unlock(&mbuf_mtx);
if ((p = (caddr_t)kmem_malloc(mb_map, nbytes, how == M_TRYWAIT ?
M_WAITOK : M_NOWAIT)) == NULL) {
mtx_lock(&mcntfree.m_mtx);
if (1 /* XXX: how == M_TRYWAIT */)
mtx_lock(&mbuf_mtx);
return (0);
}
nmb = nbytes / sizeof(union mext_refcnt);
@ -213,7 +208,8 @@ m_alloc_ref(u_int nmb, int how)
* We don't let go of the mutex in order to avoid a race.
* It is up to the caller to let go of the mutex.
*/
mtx_lock(&mcntfree.m_mtx);
if (1 /* XXX: how == M_TRYWAIT */)
mtx_lock(&mbuf_mtx);
for (i = 0; i < nmb; i++) {
((union mext_refcnt *)p)->next_ref = mcntfree.m_head;
mcntfree.m_head = (union mext_refcnt *)p;
@ -248,13 +244,15 @@ m_mballoc(int nmb, int how)
if (mb_map_full || ((nmb + mbstat.m_mbufs) > nmbufs))
return (0);
mtx_unlock(&mmbfree.m_mtx);
p = (caddr_t)kmem_malloc(mb_map, nbytes, M_NOWAIT);
if (p == NULL && how == M_TRYWAIT) {
atomic_add_long(&mbstat.m_wait, 1);
p = (caddr_t)kmem_malloc(mb_map, nbytes, M_WAITOK);
if (1 /* XXX: how == M_TRYWAIT */)
mtx_unlock(&mbuf_mtx);
p = (caddr_t)kmem_malloc(mb_map, nbytes, how == M_TRYWAIT ?
M_WAITOK : M_NOWAIT);
if (1 /* XXX: how == M_TRYWAIT */) {
mtx_lock(&mbuf_mtx);
if (p == NULL)
mbstat.m_wait++;
}
mtx_lock(&mmbfree.m_mtx);
/*
* Either the map is now full, or `how' is M_DONTWAIT and there
@ -304,15 +302,15 @@ m_mballoc_wait(void)
* importantly, to avoid a potential lock order reversal which may
* result in deadlock (See comment above m_reclaim()).
*/
mtx_unlock(&mmbfree.m_mtx);
mtx_unlock(&mbuf_mtx);
m_reclaim();
mtx_lock(&mmbfree.m_mtx);
mtx_lock(&mbuf_mtx);
_MGET(p, M_DONTWAIT);
if (p == NULL) {
m_mballoc_wid++;
msleep(&m_mballoc_wid, &mmbfree.m_mtx, PVM, "mballc",
msleep(&m_mballoc_wid, &mbuf_mtx, PVM, "mballc",
mbuf_wait);
m_mballoc_wid--;
@ -332,7 +330,7 @@ m_mballoc_wait(void)
/* If we waited and got something... */
if (p != NULL) {
atomic_add_long(&mbstat.m_wait, 1);
mbstat.m_wait++;
if (mmbfree.m_head != NULL)
MBWAKEUP(m_mballoc_wid);
}
@ -364,10 +362,12 @@ m_clalloc(int ncl, int how)
if (mb_map_full || ((ncl + mbstat.m_clusters) > nmbclusters))
return (0);
mtx_unlock(&mclfree.m_mtx);
if (1 /* XXX: how == M_TRYWAIT */)
mtx_unlock(&mbuf_mtx);
p = (caddr_t)kmem_malloc(mb_map, npg_sz,
how == M_TRYWAIT ? M_WAITOK : M_NOWAIT);
mtx_lock(&mclfree.m_mtx);
if (1 /* XXX: how == M_TRYWAIT */)
mtx_lock(&mbuf_mtx);
/*
* Either the map is now full, or `how' is M_DONTWAIT and there
@ -376,9 +376,6 @@ m_clalloc(int ncl, int how)
if (p == NULL)
return (0);
/*
* We don't let go of the mutex in order to avoid a race.
*/
for (i = 0; i < ncl; i++) {
((union mcluster *)p)->mcl_next = mclfree.m_head;
mclfree.m_head = (union mcluster *)p;
@ -403,7 +400,7 @@ m_clalloc_wait(void)
caddr_t p = NULL;
m_clalloc_wid++;
msleep(&m_clalloc_wid, &mclfree.m_mtx, PVM, "mclalc", mbuf_wait);
msleep(&m_clalloc_wid, &mbuf_mtx, PVM, "mclalc", mbuf_wait);
m_clalloc_wid--;
/*
@ -413,7 +410,7 @@ m_clalloc_wait(void)
/* If we waited and got something ... */
if (p != NULL) {
atomic_add_long(&mbstat.m_wait, 1);
mbstat.m_wait++;
if (mclfree.m_head != NULL)
MBWAKEUP(m_clalloc_wid);
}
@ -476,9 +473,8 @@ m_getclr(int how, int type)
struct mbuf *m;
MGET(m, how, type);
if (m == NULL)
return (NULL);
bzero(mtod(m, caddr_t), MLEN);
if (m != NULL)
bzero(mtod(m, caddr_t), MLEN);
return (m);
}
@ -613,8 +609,6 @@ m_prepend(struct mbuf *m, int len, int how)
* Note that the copy is read-only, because clusters are not copied,
* only their reference counts are incremented.
*/
#define MCFail (mbstat.m_mcfail)
struct mbuf *
m_copym(struct mbuf *m, int off0, int len, int wait)
{
@ -669,12 +663,17 @@ m_copym(struct mbuf *m, int off0, int len, int wait)
m = m->m_next;
np = &n->m_next;
}
if (top == NULL)
atomic_add_long(&MCFail, 1);
if (top == NULL) {
mtx_lock(&mbuf_mtx);
mbstat.m_mcfail++;
mtx_unlock(&mbuf_mtx);
}
return (top);
nospace:
m_freem(top);
atomic_add_long(&MCFail, 1);
mtx_lock(&mbuf_mtx);
mbstat.m_mcfail++;
mtx_unlock(&mbuf_mtx);
return (NULL);
}
@ -733,7 +732,9 @@ m_copypacket(struct mbuf *m, int how)
return top;
nospace:
m_freem(top);
atomic_add_long(&MCFail, 1);
mtx_lock(&mbuf_mtx);
mbstat.m_mcfail++;
mtx_unlock(&mbuf_mtx);
return (NULL);
}
@ -834,7 +835,9 @@ m_dup(struct mbuf *m, int how)
nospace:
m_freem(top);
atomic_add_long(&MCFail, 1);
mtx_lock(&mbuf_mtx);
mbstat.m_mcfail++;
mtx_unlock(&mbuf_mtx);
return (NULL);
}
@ -943,8 +946,6 @@ m_adj(struct mbuf *mp, int req_len)
* If there is room, it will add up to max_protohdr-len extra bytes to the
* contiguous region in an attempt to avoid being called next time.
*/
#define MPFail (mbstat.m_mpfail)
struct mbuf *
m_pullup(struct mbuf *n, int len)
{
@ -998,7 +999,9 @@ m_pullup(struct mbuf *n, int len)
return (m);
bad:
m_freem(n);
atomic_add_long(&MPFail, 1);
mtx_lock(&mbuf_mtx);
mbstat.m_mcfail++;
mtx_unlock(&mbuf_mtx);
return (NULL);
}

View File

@ -38,7 +38,7 @@
#define _SYS_MBUF_H_
#include <sys/lock.h>
#include <sys/mutex.h> /* XXX */
#include <sys/mutex.h>
/*
* Mbufs are of a single size, MSIZE (machine/param.h), which
@ -254,17 +254,14 @@ union mext_refcnt {
*/
struct mbffree_lst {
struct mbuf *m_head;
struct mtx m_mtx;
};
struct mclfree_lst {
union mcluster *m_head;
struct mtx m_mtx;
};
struct mcntfree_lst {
union mext_refcnt *m_head;
struct mtx m_mtx;
};
/*
@ -301,7 +298,7 @@ struct mcntfree_lst {
#define _MEXT_ALLOC_CNT(m_cnt, how) do { \
union mext_refcnt *__mcnt; \
\
mtx_lock(&mcntfree.m_mtx); \
mtx_lock(&mbuf_mtx); \
if (mcntfree.m_head == NULL) \
m_alloc_ref(1, (how)); \
__mcnt = mcntfree.m_head; \
@ -310,18 +307,18 @@ struct mcntfree_lst {
mbstat.m_refree--; \
__mcnt->refcnt = 0; \
} \
mtx_unlock(&mcntfree.m_mtx); \
mtx_unlock(&mbuf_mtx); \
(m_cnt) = __mcnt; \
} while (0)
#define _MEXT_DEALLOC_CNT(m_cnt) do { \
union mext_refcnt *__mcnt = (m_cnt); \
\
mtx_lock(&mcntfree.m_mtx); \
mtx_lock(&mbuf_mtx); \
__mcnt->next_ref = mcntfree.m_head; \
mcntfree.m_head = __mcnt; \
mbstat.m_refree++; \
mtx_unlock(&mcntfree.m_mtx); \
mtx_unlock(&mbuf_mtx); \
} while (0)
#define MEXT_INIT_REF(m, how) do { \
@ -372,15 +369,15 @@ struct mcntfree_lst {
int _mhow = (how); \
int _mtype = (type); \
\
mtx_lock(&mmbfree.m_mtx); \
mtx_lock(&mbuf_mtx); \
_MGET(_mm, _mhow); \
if (_mm != NULL) { \
mbtypes[_mtype]++; \
mtx_unlock(&mmbfree.m_mtx); \
mtx_unlock(&mbuf_mtx); \
_MGET_SETUP(_mm, _mtype); \
} else { \
mtx_unlock(&mmbfree.m_mtx); \
atomic_add_long(&mbstat.m_drops, 1); \
mbstat.m_drops++; \
mtx_unlock(&mbuf_mtx); \
} \
(m) = _mm; \
} while (0)
@ -401,15 +398,15 @@ struct mcntfree_lst {
int _mhow = (how); \
int _mtype = (type); \
\
mtx_lock(&mmbfree.m_mtx); \
mtx_lock(&mbuf_mtx); \
_MGET(_mm, _mhow); \
if (_mm != NULL) { \
mbtypes[_mtype]++; \
mtx_unlock(&mmbfree.m_mtx); \
mtx_unlock(&mbuf_mtx); \
_MGETHDR_SETUP(_mm, _mtype); \
} else { \
mtx_unlock(&mmbfree.m_mtx); \
atomic_add_long(&mbstat.m_drops, 1); \
mbstat.m_drops++; \
mtx_unlock(&mbuf_mtx); \
} \
(m) = _mm; \
} while (0)
@ -442,10 +439,10 @@ struct mcntfree_lst {
#define MCLGET(m, how) do { \
struct mbuf *_mm = (m); \
\
mtx_lock(&mclfree.m_mtx); \
mtx_lock(&mbuf_mtx); \
_MCLALLOC(_mm->m_ext.ext_buf, (how)); \
mtx_unlock(&mclfree.m_mtx); \
if (_mm->m_ext.ext_buf != NULL) { \
mtx_unlock(&mbuf_mtx); \
MEXT_INIT_REF(_mm, (how)); \
if (_mm->m_ext.ref_cnt == NULL) { \
_MCLFREE(_mm->m_ext.ext_buf); \
@ -458,8 +455,10 @@ struct mcntfree_lst {
_mm->m_ext.ext_size = MCLBYTES; \
_mm->m_ext.ext_type = EXT_CLUSTER; \
} \
} else \
atomic_add_long(&mbstat.m_drops, 1); \
} else { \
mbstat.m_drops++; \
mtx_unlock(&mbuf_mtx); \
} \
} while (0)
#define MEXTADD(m, buf, size, free, args, flags, type) do { \
@ -480,12 +479,12 @@ struct mcntfree_lst {
#define _MCLFREE(p) do { \
union mcluster *_mp = (union mcluster *)(p); \
\
mtx_lock(&mclfree.m_mtx); \
mtx_lock(&mbuf_mtx); \
_mp->mcl_next = mclfree.m_head; \
mclfree.m_head = _mp; \
mbstat.m_clfree++; \
MBWAKEUP(m_clalloc_wid); \
mtx_unlock(&mclfree.m_mtx); \
mtx_unlock(&mbuf_mtx); \
} while (0)
/* MEXTFREE:
@ -520,7 +519,7 @@ struct mcntfree_lst {
KASSERT(_mm->m_type != MT_FREE, ("freeing free mbuf")); \
if (_mm->m_flags & M_EXT) \
MEXTFREE(_mm); \
mtx_lock(&mmbfree.m_mtx); \
mtx_lock(&mbuf_mtx); \
mbtypes[_mm->m_type]--; \
_mm->m_type = MT_FREE; \
mbtypes[MT_FREE]++; \
@ -528,7 +527,7 @@ struct mcntfree_lst {
_mm->m_next = mmbfree.m_head; \
mmbfree.m_head = _mm; \
MBWAKEUP(m_mballoc_wid); \
mtx_unlock(&mmbfree.m_mtx); \
mtx_unlock(&mbuf_mtx); \
} while (0)
/*
@ -619,8 +618,10 @@ struct mcntfree_lst {
struct mbuf *_mm = (m); \
int _mt = (t); \
\
atomic_subtract_long(&mbtypes[_mm->m_type], 1); \
atomic_add_long(&mbtypes[_mt], 1); \
mtx_lock(&mbuf_mtx); \
mbtypes[_mm->m_type]--; \
mbtypes[_mt]++; \
mtx_unlock(&mbuf_mtx); \
_mm->m_type = (_mt); \
} while (0)
@ -652,6 +653,7 @@ extern struct mbuf *mbutl; /* virtual address of mclusters */
extern struct mclfree_lst mclfree;
extern struct mbffree_lst mmbfree;
extern struct mcntfree_lst mcntfree;
extern struct mtx mbuf_mtx;
extern int nmbclusters;
extern int nmbufs;
extern int nsfbufs;