Date: Thu, 8 Jul 2010 19:50:31 GMT From: Edward Tomasz Napierala <trasz@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 180661 for review Message-ID: <201007081950.o68JoVKj000453@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://p4web.freebsd.org/@@180661?ac=10 Change 180661 by trasz@trasz_victim on 2010/07/08 19:50:29 Make HRL use its own mutex instead of container_lock. Also, make container_lock non-recursive. Affected files ... .. //depot/projects/soc2009/trasz_limits/sys/kern/kern_container.c#12 edit .. //depot/projects/soc2009/trasz_limits/sys/kern/kern_hrl.c#82 edit Differences ... ==== //depot/projects/soc2009/trasz_limits/sys/kern/kern_container.c#12 (text+ko) ==== @@ -59,8 +59,8 @@ #include <sys/hrl.h> #endif -struct mtx container_lock; -MTX_SYSINIT(container_lock, &container_lock, "container lock", MTX_RECURSE); /* XXX: Make it non-recurseable later. */ +static struct mtx container_lock; +MTX_SYSINIT(container_lock, &container_lock, "container lock", MTX_DEF); static int container_add(struct container *dest, const struct container *src) @@ -112,8 +112,8 @@ } } -int -container_join(struct container *child, struct container *parent) +static int +container_join_locked(struct container *child, struct container *parent) { int i, error; @@ -136,8 +136,20 @@ panic("container has too many parents"); } -void -container_leave(struct container *child, struct container *parent) +int +container_join(struct container *child, struct container *parent) +{ + int error; + + mtx_lock(&container_lock); + error = container_join_locked(child, parent); + mtx_unlock(&container_lock); + + return (error); +} + +static void +container_leave_locked(struct container *child, struct container *parent) { int i; @@ -155,6 +167,15 @@ panic("container not joined"); } +void +container_leave(struct container *child, struct container *parent) +{ + + mtx_lock(&container_lock); + container_leave_locked(child, parent); + mtx_unlock(&container_lock); +} + static void container_leave_parents(struct container *child) { @@ -184,12 +205,14 @@ ("container->c_parents[%d] != NULL", i)); } -void -container_destroy(struct container *container) +static void +container_destroy_locked(struct container *container) { int i; - mtx_lock(&container_lock); + mtx_assert(&container_lock, MA_OWNED); + KASSERT(container != NULL, ("NULL container")); + for (i = 0; i <= RUSAGE_MAX; i++) { if (container->c_resources[i] != 0) printf("destroying non-empty container: " @@ -199,6 +222,14 @@ } container_leave_parents(container); +} + +void +container_destroy(struct container *container) +{ + + mtx_lock(&container_lock); + container_destroy_locked(container); mtx_unlock(&container_lock); } @@ -291,15 +322,8 @@ return (0); } -/* - * Set allocation of 'resource' to 'amount' for process 'p'. - * Return 0 if it's below limits, or errno, if it's not. - * - * Note that decreasing the allocation always returns 0, - * even if it's above the limit. - */ -int -rusage_set(struct proc *p, int resource, uint64_t amount) +static int +rusage_set_locked(struct proc *p, int resource, uint64_t amount) { int64_t diff; #ifdef HRL @@ -313,7 +337,6 @@ KASSERT(amount >= 0, ("rusage_set: invalid amount for resource %d: %ju", resource, amount)); - mtx_lock(&container_lock); diff = amount - p->p_container.c_resources[resource]; #ifdef HRL if (diff > 0) { @@ -325,12 +348,30 @@ } #endif container_alloc_resource(&p->p_container, resource, diff); - mtx_unlock(&container_lock); return (0); } /* + * Set allocation of 'resource' to 'amount' for process 'p'. + * Return 0 if it's below limits, or errno, if it's not. + * + * Note that decreasing the allocation always returns 0, + * even if it's above the limit. + */ +int +rusage_set(struct proc *p, int resource, uint64_t amount) +{ + int error; + + mtx_lock(&container_lock); + error = rusage_set_locked(p, resource, amount); + mtx_unlock(&container_lock); + + return (error); +} + +/* * Decrease allocation of 'resource' by 'amount' for process 'p'. */ void @@ -376,12 +417,10 @@ rusage_set(p, RUSAGE_COREDUMPSIZE, 0); rusage_set(p, RUSAGE_PTY, 0); - mtx_lock(&container_lock); #ifdef HRL hrl_proc_exit(p); #endif container_destroy(&p->p_container); - mtx_unlock(&container_lock); } /* @@ -412,15 +451,15 @@ !container_resource_inheritable(i)) continue; - error = rusage_set(child, i, parent->p_container.c_resources[i]); + error = rusage_set_locked(child, i, parent->p_container.c_resources[i]); if (error) { /* * XXX: The only purpose of these two lines is to prevent from * tripping checks in container_destroy(). */ for (i = 0; i <= RUSAGE_MAX; i++) - rusage_set(child, i, 0); - container_destroy(&child->p_container); + rusage_set_locked(child, i, 0); + container_destroy_locked(&child->p_container); goto out; } } @@ -432,31 +471,34 @@ container = parent->p_container.c_parents[i]; if (container == NULL) continue; - error = container_join(&child->p_container, container); + error = container_join_locked(&child->p_container, container); if (error) { /* * XXX: The only purpose of these two lines is to prevent from * tripping checks in container_destroy(). */ for (i = 0; i <= RUSAGE_MAX; i++) - rusage_set(child, i, 0); - container_destroy(&child->p_container); + rusage_set_locked(child, i, 0); + container_destroy_locked(&child->p_container); goto out; } } -#ifdef HRL - error = hrl_proc_fork(parent, child); - if (error) { - container_destroy(&child->p_container); - goto out; - } -#endif - out: mtx_unlock(&container_lock); PROC_UNLOCK(child); PROC_UNLOCK(parent); +#ifdef HRL + if (error == 0) { + error = hrl_proc_fork(parent, child); + if (error) { + mtx_lock(&container_lock); + container_destroy(&child->p_container); + mtx_unlock(&container_lock); + } + } +#endif + return (error); } ==== //depot/projects/soc2009/trasz_limits/sys/kern/kern_hrl.c#82 (text+ko) ==== @@ -120,7 +120,8 @@ static uma_zone_t hrl_rule_link_zone; static uma_zone_t hrl_rule_zone; -extern struct mtx container_lock; +static struct mtx hrl_lock; +MTX_SYSINIT(hrl_lock, &hrl_lock, "HRL lock", MTX_RECURSE); /* XXX: Make it non-recurseable later. */ static void hrl_compute_available(struct proc *p, int64_t (*availablep)[]); static int hrl_rule_fully_specified(const struct hrl_rule *rule); @@ -196,7 +197,7 @@ int64_t available = INT64_MAX; struct ucred *cred = p->p_ucred; - mtx_assert(&container_lock, MA_OWNED); + mtx_assert(&hrl_lock, MA_OWNED); resource = rule->hr_resource; switch (rule->hr_per) { @@ -235,7 +236,7 @@ { int64_t available; - mtx_assert(&container_lock, MA_OWNED); + mtx_assert(&hrl_lock, MA_OWNED); available = hrl_available_resource(p, rule); if (available >= amount) @@ -270,15 +271,17 @@ int should_deny = 0; char *buf; - mtx_assert(&container_lock, MA_OWNED); + mtx_lock(&hrl_lock); /* * XXX: Do this just before we start running on a CPU, not all the time. */ hrl_compute_available(p, &available); - if (available[resource] >= amount) + if (available[resource] >= amount) { + mtx_unlock(&hrl_lock); return (0); + } /* * It seems we've hit a limit. Figure out what to do. There may @@ -344,6 +347,8 @@ } } + mtx_unlock(&hrl_lock); + if (should_deny) { /* * Return fake error code; the caller should change it @@ -367,7 +372,7 @@ struct hrl_rule_link *link; struct hrl_rule *rule; - mtx_assert(&container_lock, MA_OWNED); + mtx_assert(&hrl_lock, MA_OWNED); for (i = 0; i <= RUSAGE_MAX; i++) (*availablep)[i] = INT64_MAX; @@ -513,9 +518,9 @@ link = uma_zalloc(hrl_rule_link_zone, M_WAITOK); link->hrl_rule = rule; - mtx_lock(&container_lock); + mtx_lock(&hrl_lock); LIST_INSERT_HEAD(&container->c_rule_links, link, hrl_next); - mtx_unlock(&container_lock); + mtx_unlock(&hrl_lock); } static int @@ -524,7 +529,7 @@ struct hrl_rule_link *link; KASSERT(hrl_rule_fully_specified(rule), ("rule not fully specified")); - mtx_assert(&container_lock, MA_OWNED); + mtx_assert(&hrl_lock, MA_OWNED); link = uma_zalloc(hrl_rule_link_zone, M_NOWAIT); if (link == NULL) @@ -548,7 +553,7 @@ int removed = 0; struct hrl_rule_link *link, *linktmp; - mtx_lock(&container_lock); + mtx_lock(&hrl_lock); LIST_FOREACH_SAFE(link, &container->c_rule_links, hrl_next, linktmp) { if (!hrl_rule_matches(link->hrl_rule, filter)) continue; @@ -558,7 +563,7 @@ uma_zfree(hrl_rule_link_zone, link); removed++; } - mtx_unlock(&container_lock); + mtx_unlock(&hrl_lock); return (removed); } @@ -1214,7 +1219,7 @@ struct hrl_rule_link *link; struct sbuf *sb = (struct sbuf *)arg3; - mtx_assert(&container_lock, MA_OWNED); + mtx_assert(&hrl_lock, MA_OWNED); LIST_FOREACH(link, &container->c_rule_links, hrl_next) { if (!hrl_rule_matches(link->hrl_rule, filter)) @@ -1256,7 +1261,7 @@ sx_assert(&allproc_lock, SA_LOCKED); FOREACH_PROC_IN_SYSTEM(p) { - mtx_lock(&container_lock); + mtx_lock(&hrl_lock); LIST_FOREACH(link, &p->p_container.c_rule_links, hrl_next) { /* * Non-process rules will be added to the buffer later. @@ -1269,14 +1274,14 @@ hrl_rule_to_sbuf(sb, link->hrl_rule); sbuf_printf(sb, ","); } - mtx_unlock(&container_lock); + mtx_unlock(&hrl_lock); } - mtx_lock(&container_lock); + mtx_lock(&hrl_lock); loginclass_container_foreach(hrl_get_rules_callback, filter, sb); ui_container_foreach(hrl_get_rules_callback, filter, sb); gi_container_foreach(hrl_get_rules_callback, filter, sb); - mtx_unlock(&container_lock); + mtx_unlock(&hrl_lock); if (sbuf_overflowed(sb)) { sbuf_delete(sb); free(buf, M_HRL); @@ -1341,12 +1346,12 @@ sb = sbuf_new(NULL, buf, bufsize, SBUF_FIXEDLEN); KASSERT(sb != NULL, ("sbuf_new failed")); - mtx_lock(&container_lock); + mtx_lock(&hrl_lock); LIST_FOREACH(link, &filter->hr_subject.hs_proc->p_container.c_rule_links, hrl_next) { hrl_rule_to_sbuf(sb, link->hrl_rule); sbuf_printf(sb, ","); } - mtx_unlock(&container_lock); + mtx_unlock(&hrl_lock); if (sbuf_overflowed(sb)) { sbuf_delete(sb); free(buf, M_HRL); @@ -1448,8 +1453,6 @@ int error; struct ucred *cred = p->p_ucred; - mtx_lock(&container_lock); - container_create(&p->p_container); error = container_join(&p->p_container, &cred->cr_ruidinfo->ui_container); KASSERT(error == 0, ("hrl_proc_init: container_join failed")); @@ -1457,8 +1460,6 @@ KASSERT(error == 0, ("hrl_proc_init: container_join failed")); error = container_join(&p->p_container, &cred->cr_prison->pr_container); KASSERT(error == 0, ("hrl_proc_init: container_join failed")); - - mtx_unlock(&container_lock); } /* @@ -1483,11 +1484,10 @@ newpr = newcred->cr_prison; oldpr = p->p_ucred->cr_prison; - mtx_lock(&container_lock); - /* * Remove rules that are no longer applicable with the new ucred. */ + mtx_lock(&hrl_lock); LIST_FOREACH(link, &p->p_container.c_rule_links, hrl_next) { switch (link->hrl_rule->hr_subject_type) { case HRL_SUBJECT_TYPE_PROCESS: @@ -1513,42 +1513,47 @@ hrl_rule_release(link->hrl_rule); uma_zfree(hrl_rule_link_zone, link); } + mtx_unlock(&hrl_lock); /* * Add rules for the new ucred and move between containers where applicable. */ if (newuip != olduip) { + mtx_lock(&hrl_lock); LIST_FOREACH(link, &newuip->ui_container.c_rule_links, hrl_next) { error = hrl_container_add_rule_locked(&p->p_container, link->hrl_rule); KASSERT(error == 0, ("XXX: better error handling needed")); } + mtx_unlock(&hrl_lock); container_leave(&p->p_container, &olduip->ui_container); error = container_join(&p->p_container, &newuip->ui_container); KASSERT(error == 0, ("hrl_proc_ucred_changing: better error handling needed")); } if (newlc != oldlc) { + mtx_lock(&hrl_lock); LIST_FOREACH(link, &newlc->lc_container.c_rule_links, hrl_next) { error = hrl_container_add_rule_locked(&p->p_container, link->hrl_rule); KASSERT(error == 0, ("XXX: better error handling needed")); } + mtx_unlock(&hrl_lock); container_leave(&p->p_container, &oldlc->lc_container); error = container_join(&p->p_container, &newlc->lc_container); KASSERT(error == 0, ("hrl_proc_ucred_changing: better error handling needed")); } if (newpr != oldpr) { + mtx_lock(&hrl_lock); LIST_FOREACH(link, &newpr->pr_container.c_rule_links, hrl_next) { error = hrl_container_add_rule_locked(&p->p_container, link->hrl_rule); KASSERT(error == 0, ("XXX: better error handling needed")); } + mtx_unlock(&hrl_lock); container_leave(&p->p_container, &oldpr->pr_container); error = container_join(&p->p_container, &newpr->pr_container); KASSERT(error == 0, ("hrl_proc_ucred_changing: better error handling needed")); } - - mtx_unlock(&container_lock); } /* @@ -1561,7 +1566,7 @@ struct hrl_rule_link *link; struct hrl_rule *rule; - mtx_lock(&container_lock); + mtx_lock(&hrl_lock); /* * Go through limits applicable to the parent and assign them to the child. @@ -1587,7 +1592,7 @@ } } - mtx_unlock(&container_lock); + mtx_unlock(&hrl_lock); return (0); fail: @@ -1597,7 +1602,7 @@ hrl_rule_release(link->hrl_rule); uma_zfree(hrl_rule_link_zone, link); } - mtx_unlock(&container_lock); + mtx_unlock(&hrl_lock); return (EAGAIN); } @@ -1609,14 +1614,14 @@ { struct hrl_rule_link *link; - mtx_lock(&container_lock); + mtx_lock(&hrl_lock); while (!LIST_EMPTY(&p->p_container.c_rule_links)) { link = LIST_FIRST(&p->p_container.c_rule_links); LIST_REMOVE(link, hrl_next); hrl_rule_release(link->hrl_rule); uma_zfree(hrl_rule_link_zone, link); } - mtx_unlock(&container_lock); + mtx_unlock(&hrl_lock); } static void
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201007081950.o68JoVKj000453>