Changeset View
Standalone View
sys/rpc/svc.c
Show First 20 Lines • Show All 107 Lines • ▼ Show 20 Lines | for (g = 0; g < SVC_MAXGROUPS; g++) { | ||||
TAILQ_INIT(&grp->sg_xlist); | TAILQ_INIT(&grp->sg_xlist); | ||||
TAILQ_INIT(&grp->sg_active); | TAILQ_INIT(&grp->sg_active); | ||||
LIST_INIT(&grp->sg_idlethreads); | LIST_INIT(&grp->sg_idlethreads); | ||||
grp->sg_minthreads = 1; | grp->sg_minthreads = 1; | ||||
grp->sg_maxthreads = 1; | grp->sg_maxthreads = 1; | ||||
} | } | ||||
/* | /* | ||||
* Don't use more than a quarter of mbuf clusters or more than | * Don't use more than a quarter of mbuf clusters. | ||||
* 45Mb buffering requests. | |||||
*/ | */ | ||||
pool->sp_space_high = nmbclusters * MCLBYTES / 4; | pool->sp_space_high = nmbclusters * MCLBYTES / 4; | ||||
if (pool->sp_space_high > 45 << 20) | |||||
pool->sp_space_high = 45 << 20; | |||||
pool->sp_space_low = 2 * pool->sp_space_high / 3; | pool->sp_space_low = 2 * pool->sp_space_high / 3; | ||||
wollman: clang gives a tautological-compare warning here on LP64 platforms (nmbclusters is an int). | |||||
sysctl_ctx_init(&pool->sp_sysctl); | sysctl_ctx_init(&pool->sp_sysctl); | ||||
if (sysctl_base) { | if (sysctl_base) { | ||||
SYSCTL_ADD_PROC(&pool->sp_sysctl, sysctl_base, OID_AUTO, | SYSCTL_ADD_PROC(&pool->sp_sysctl, sysctl_base, OID_AUTO, | ||||
"minthreads", CTLTYPE_INT | CTLFLAG_RW, | "minthreads", CTLTYPE_INT | CTLFLAG_RW, | ||||
pool, 0, svcpool_minthread_sysctl, "I", | pool, 0, svcpool_minthread_sysctl, "I", | ||||
"Minimal number of threads"); | "Minimal number of threads"); | ||||
SYSCTL_ADD_PROC(&pool->sp_sysctl, sysctl_base, OID_AUTO, | SYSCTL_ADD_PROC(&pool->sp_sysctl, sysctl_base, OID_AUTO, | ||||
▲ Show 20 Lines • Show All 1,120 Lines • ▼ Show 20 Lines | while (grp->sg_state != SVCPOOL_CLOSING) { | ||||
while ((rqstp = STAILQ_FIRST(&st->st_reqs)) != NULL) { | while ((rqstp = STAILQ_FIRST(&st->st_reqs)) != NULL) { | ||||
STAILQ_REMOVE_HEAD(&st->st_reqs, rq_link); | STAILQ_REMOVE_HEAD(&st->st_reqs, rq_link); | ||||
mtx_unlock(&st->st_lock); | mtx_unlock(&st->st_lock); | ||||
sz += rqstp->rq_size; | sz += rqstp->rq_size; | ||||
svc_executereq(rqstp); | svc_executereq(rqstp); | ||||
mtx_lock(&st->st_lock); | mtx_lock(&st->st_lock); | ||||
} | } | ||||
mtx_unlock(&st->st_lock); | mtx_unlock(&st->st_lock); | ||||
svc_change_space_used(pool, -sz); | svc_change_space_used(pool, -sz); | ||||
Not Done Inline ActionsPreviously, sz was a size_t, which is unsigned long, and -sz (still unsigned long) was passed to a signed int formal parameter, resulting in overflow and undefined behavior. I suspect gcc 4.2 did the "obvious" thing and clang generates different code here that doesn't. wollman: Previously, sz was a size_t, which is unsigned long, and -sz (still unsigned long) was passed… | |||||
Not Done Inline ActionsHere's how GCC implemented it in my 9.3 kernel: 0xffffffff807bd50e <+1454>: mov %r13d,%esi 0xffffffff807bd511 <+1457>: mov %r14,%rdi 0xffffffff807bd514 <+1460>: neg %esi 0xffffffff807bd516 <+1462>: callq 0xffffffff807bc820 <svc_change_space_used> It actually performs the negation in the parameter type (int) rather than the value type (size_t). Of course any situation in which the result would be different involves undefined behavior so GCC was not in error here (the code was still wrong). wollman: Here's how GCC implemented it in my 9.3 kernel:
0xffffffff807bd50e <+1454>: mov %r13d… | |||||
Not Done Inline ActionsThinking that from the behavior I'm seeing, this should perhaps be hoisted up into the loop, so that the space-in-use is immediately credited when the request competes. We don't batch this adjustment on insert, so I don't think it should be batched on dequeue either. Thoughts? wollman: Thinking that from the behavior I'm seeing, this should perhaps be hoisted up into the loop, so… | |||||
mtx_lock(&grp->sg_lock); | mtx_lock(&grp->sg_lock); | ||||
} | } | ||||
if (st->st_xprt) { | if (st->st_xprt) { | ||||
xprt = st->st_xprt; | xprt = st->st_xprt; | ||||
st->st_xprt = NULL; | st->st_xprt = NULL; | ||||
SVC_RELEASE(xprt); | SVC_RELEASE(xprt); | ||||
} | } | ||||
▲ Show 20 Lines • Show All 155 Lines • Show Last 20 Lines |
clang gives a tautological-compare warning here on LP64 platforms (nmbclusters is an int). What's the test I really should be doing?