From owner-p4-projects@FreeBSD.ORG Sun Jun 21 20:30:42 2009 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id F10B81065687; Sun, 21 Jun 2009 20:30:41 +0000 (UTC) Delivered-To: perforce@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B09CB106566C for ; Sun, 21 Jun 2009 20:30:41 +0000 (UTC) (envelope-from trasz@freebsd.org) Received: from repoman.freebsd.org (repoman.freebsd.org [IPv6:2001:4f8:fff6::29]) by mx1.freebsd.org (Postfix) with ESMTP id 9DD508FC19 for ; Sun, 21 Jun 2009 20:30:41 +0000 (UTC) (envelope-from trasz@freebsd.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.3/8.14.3) with ESMTP id n5LKUf6D064890 for ; Sun, 21 Jun 2009 20:30:41 GMT (envelope-from trasz@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.3/8.14.3/Submit) id n5LKUfjf064888 for perforce@freebsd.org; Sun, 21 Jun 2009 20:30:41 GMT (envelope-from trasz@freebsd.org) Date: Sun, 21 Jun 2009 20:30:41 GMT Message-Id: <200906212030.n5LKUfjf064888@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to trasz@freebsd.org using -f From: Edward Tomasz Napierala To: Perforce Change Reviews Cc: Subject: PERFORCE change 164828 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 21 Jun 2009 20:30:42 -0000 http://perforce.freebsd.org/chv.cgi?CH=164828 Change 164828 by trasz@trasz_victim on 2009/06/21 20:29:52 Trying to get hierarchical resource accounting to work. This still trips on KASSERTs horribly, for some reason. I'm running out of ideas. ;-/ Affected files ... .. //depot/projects/soc2009/trasz_limits/sys/kern/kern_exec.c#4 edit .. //depot/projects/soc2009/trasz_limits/sys/kern/kern_exit.c#7 edit .. //depot/projects/soc2009/trasz_limits/sys/kern/kern_fork.c#5 edit .. //depot/projects/soc2009/trasz_limits/sys/kern/kern_hrl.c#14 edit .. //depot/projects/soc2009/trasz_limits/sys/sys/hrl.h#10 edit .. //depot/projects/soc2009/trasz_limits/sys/sys/ucred.h#5 edit Differences ... ==== //depot/projects/soc2009/trasz_limits/sys/kern/kern_exec.c#4 (text+ko) ==== @@ -705,7 +705,7 @@ */ change_svuid(newcred, newcred->cr_uid); change_svgid(newcred, newcred->cr_gid); - p->p_ucred = newcred; + change_cred(p, newcred); newcred = NULL; } else { if (oldcred->cr_uid == oldcred->cr_ruid && @@ -728,7 +728,7 @@ crcopy(newcred, oldcred); change_svuid(newcred, newcred->cr_uid); change_svgid(newcred, newcred->cr_gid); - p->p_ucred = newcred; + change_cred(p, newcred); newcred = NULL; } } ==== //depot/projects/soc2009/trasz_limits/sys/kern/kern_exit.c#7 (text+ko) ==== @@ -128,7 +128,7 @@ struct ucred *tracecred; #endif struct plimit *plim; - int locked, i; + int locked; mtx_assert(&Giant, MA_NOTOWNED); @@ -398,16 +398,6 @@ PROC_UNLOCK(p); lim_free(plim); - for (i = 0; i < HRL_RESOURCE_MAX; i++) { - if (p->p_accounting.ha_resources[i] != 0) -#ifdef notyet - printf("exit1: exiting process: resource %d = %lld\n", - i, p->p_accounting.ha_resources[i]); -#else - ; -#endif - } - /* * Remove proc from allproc queue and pidhash chain. * Place onto zombproc. Unlink from parent's child list. @@ -772,6 +762,8 @@ * Decrement the count of procs running with this uid. */ (void)chgproccnt(p->p_ucred->cr_ruidinfo, -1, 0); + hrl_free_proc(p->p_pptr, HRL_RESOURCE_MAXPROCESSES, 1); + hrl_proc_exiting(p); /* * Free credentials, arguments, and sigacts. @@ -934,6 +926,10 @@ if (child->p_pptr == parent) return; + hrl_free_proc(child->p_pptr, HRL_RESOURCE_MAXPROCESSES, 1); + /* XXX: What about return value? */ + hrl_alloc_proc(parent, HRL_RESOURCE_MAXPROCESSES, 1); + PROC_LOCK(child->p_pptr); sigqueue_take(child->p_ksi); PROC_UNLOCK(child->p_pptr); ==== //depot/projects/soc2009/trasz_limits/sys/kern/kern_fork.c#5 (text+ko) ==== @@ -570,6 +570,12 @@ PROC_UNLOCK(p1); PROC_UNLOCK(p2); + /* + * Initialize HRL accounting information. + * XXX: Handle failure? + */ + hrl_proc_fork(p1, p2); + /* Bump references to the text vnode (for procfs) */ if (p2->p_textvp) vref(p2->p_textvp); ==== //depot/projects/soc2009/trasz_limits/sys/kern/kern_hrl.c#14 (text+ko) ==== @@ -47,6 +47,11 @@ #include #include +#define HRF_DEFAULT 0 +#define HRF_DONT_INHERIT 1 +#define HRF_DONT_ACCUMULATE 2 + + struct hrl_node { struct hrl_rule hn_rule; RB_ENTRY(hrl_node) hn_next; @@ -93,14 +98,24 @@ RB_HEAD(hrl_tree, hrl_node) hrls = RB_INITIALIZER(hrls); RB_GENERATE_STATIC(hrl_tree, hrl_node, hn_next, hn_compare); +static const char * hrl_resource_name(int resource); static void hrl_init(void); -SYSINIT(hrl, SI_SUB_RUN_SCHEDULER, SI_ORDER_SECOND, hrl_init, NULL); +SYSINIT(hrl, SI_SUB_CPU, SI_ORDER_FIRST, hrl_init, NULL); static uma_zone_t hrl_zone; static struct mtx hrl_lock; MALLOC_DEFINE(M_HRL, "hrl", "Hierarchical Resource Limits"); +#define notyet +#if 0 +#undef KASSERT +#define KASSERT(exp,msg) do { \ + if (__predict_false(!(exp))) \ + printf msg; \ +} while (0) +#endif + #ifdef INVARIANTS /* * Go through the accounting info and verify that it makes sense. @@ -114,7 +129,7 @@ struct prison *pr; cred = p->p_ucred; - + mtx_assert(&hrl_lock, MA_OWNED); for (resource = 0; resource < HRL_RESOURCE_MAX; resource++) KASSERT(p->p_accounting.ha_resources[resource] >= 0, ("resource usage propagation meltdown")); KASSERT(cred->cr_ruidinfo->ui_accounting.ha_resources[resource] >= 0, ("resource usage propagation meltdown")); @@ -128,6 +143,88 @@ } #endif /* INVARIANTS */ +void +hrl_proc_exiting(struct proc *p) +{ + int i; + + mtx_lock(&hrl_lock); + for (i = 0; i < HRL_RESOURCE_MAX; i++) { + if (p->p_accounting.ha_resources[i] != 0) +#if 0 + KASSERT(p->p_accounting.ha_resources == 0, + ("dead process still holding resources")); +#else + printf("hrl_proc_exiting: %s = %lld\n", + hrl_resource_name(i), + p->p_accounting.ha_resources[i]); + if (p->p_accounting.ha_resources[i] > 0) + hrl_free_proc(p, i, p->p_accounting.ha_resources[i]); + else + p->p_accounting.ha_resources[i] = 0; +#endif + } + mtx_unlock(&hrl_lock); +} + +static int +hrl_resource_inheritable(int resource) +{ + + switch (resource) { + case HRL_RESOURCE_MAXPROCESSES: + return (0); + default: + return (1); + } +} + +static const char * +hrl_resource_name(int resource) +{ + switch (resource) { + case HRL_RESOURCE_CPUTIME: + return ("cputime"); + case HRL_RESOURCE_FILESIZE: + return ("filesize"); + case HRL_RESOURCE_DATASIZE: + return ("datasize"); + case HRL_RESOURCE_STACKSIZE: + return ("stacksize"); + case HRL_RESOURCE_COREDUMPSIZE: + return ("coredumpsize"); + case HRL_RESOURCE_MEMORYUSE: + return ("memoryuse"); + case HRL_RESOURCE_MEMORYLOCKED: + return ("memorylocked"); + case HRL_RESOURCE_MAXPROCESSES: + return ("maxprocesses"); + case HRL_RESOURCE_OPENFILES: + return ("openfiles"); + case HRL_RESOURCE_SBSIZE: + return ("sbsize"); + case HRL_RESOURCE_VMEMORYUSE: + return ("vmemoryuse"); + case HRL_RESOURCE_PTY: + return ("vmemoryuse"); + default: + panic("hrl_resource_name: unknown resource"); + } +} + +void +hrl_proc_fork(struct proc *parent, struct proc *child) +{ + int i; + + mtx_lock(&hrl_lock); + for (i = 0; i < HRL_RESOURCE_MAX; i++) { + if (parent->p_accounting.ha_resources[i] != 0 && hrl_resource_inheritable(i)) + hrl_allocated_proc(child, i, parent->p_accounting.ha_resources[i]); + } + mtx_unlock(&hrl_lock); +} + /* * Increase allocation of 'resource' by 'amount' for process 'p'. * Return 0 if it's below limits, or errno, if it's not. @@ -135,27 +232,43 @@ int hrl_alloc_proc(struct proc *p, int resource, uint64_t amount) { - int i; + int i, j; struct ucred *cred; struct prison *pr; - KASSERT(amount > 0, ("invalid amount")); + KASSERT(amount > 0, ("hrl_alloc_proc: invalid amount for %s: %lld", + hrl_resource_name(resource), amount)); + if (amount <= 0) + panic("bleh."); - /* - * XXX: Obviously wrong, fix later. - */ + mtx_lock(&hrl_lock); p->p_accounting.ha_resources[resource] += amount; cred = p->p_ucred; cred->cr_ruidinfo->ui_accounting.ha_resources[resource] += amount; - cred->cr_uidinfo->ui_accounting.ha_resources[resource] += amount; + if (cred->cr_ruidinfo != cred->cr_uidinfo) + cred->cr_uidinfo->ui_accounting.ha_resources[resource] += amount; for (pr = cred->cr_prison; pr != NULL; pr = pr->pr_parent) pr->pr_accounting.ha_resources[resource] += amount; - for (i = 0; i < cred->cr_ngroups; i++) + /* + * XXX: Slow. + */ + for (i = 0; i < cred->cr_ngroups; i++) { + /* + * Make sure we don't account a group more than once if it appears + * in cr_groups[] more than once. + */ + for (j = 0; j < i; j++) { + if (cred->cr_groups[i] == cred->cr_groups[j]) + goto skip_group; + } cred->cr_gidinfos[i]->gi_accounting.ha_resources[resource] += amount; - +skip_group: + continue; + } #ifdef INVARIANTS hrl_assert_proc(p); #endif + mtx_unlock(&hrl_lock); /* * XXX: When denying, return proper errno - EFSIZ, ENOMEM etc. @@ -174,24 +287,45 @@ int hrl_allocated_proc(struct proc *p, int resource, uint64_t amount) { - int i; + int i, j; int64_t diff; struct ucred *cred; struct prison *pr; + KASSERT(amount > 0, ("hrl_allocated_proc: invalid amount for %s: %lld", + hrl_resource_name(resource), amount)); + if (amount <= 0) + panic("bleh."); + + mtx_lock(&hrl_lock); diff = amount - p->p_accounting.ha_resources[resource]; - p->p_accounting.ha_resources[resource] += diff; + p->p_accounting.ha_resources[resource] = amount; cred = p->p_ucred; cred->cr_ruidinfo->ui_accounting.ha_resources[resource] += diff; - cred->cr_uidinfo->ui_accounting.ha_resources[resource] += diff; + if (cred->cr_ruidinfo != cred->cr_uidinfo) + cred->cr_uidinfo->ui_accounting.ha_resources[resource] += diff; for (pr = cred->cr_prison; pr != NULL; pr = pr->pr_parent) pr->pr_accounting.ha_resources[resource] += diff; - for (i = 0; i < cred->cr_ngroups; i++) + /* + * XXX: Slow. + */ + for (i = 0; i < cred->cr_ngroups; i++) { + /* + * Make sure we don't account a group more than once if it appears + * in cr_groups[] more than once. + */ + for (j = 0; j < i; j++) { + if (cred->cr_groups[i] == cred->cr_groups[j]) + goto skip_group; + } cred->cr_gidinfos[i]->gi_accounting.ha_resources[resource] += diff; - +skip_group: + continue; + } #ifdef INVARIANTS hrl_assert_proc(p); #endif + mtx_unlock(&hrl_lock); return (0); } @@ -202,27 +336,49 @@ void hrl_free_proc(struct proc *p, int resource, uint64_t amount) { - int i; + int i, j; struct ucred *cred; struct prison *pr; - KASSERT(amount > 0, ("invalid amount")); + KASSERT(amount > 0, ("hrl_free_proc: invalid amount for %s: %lld", + hrl_resource_name(resource), amount)); + if (amount <= 0) + panic("bleh."); + mtx_lock(&hrl_lock); p->p_accounting.ha_resources[resource] -= amount; #ifdef notyet - KASSERT(amount >= p->p_accounting.ha_resources[resource], ("freeing more than allocated")); + KASSERT(amount <= p->p_accounting.ha_resources[resource], + ("hrl_free_proc: freeing %lld, which is more than allocated %lld " + "for %s", amount, p->p_accounting.ha_resources[resource], + hrl_resource_name(resource))); #endif cred = p->p_ucred; cred->cr_ruidinfo->ui_accounting.ha_resources[resource] -= amount; - cred->cr_uidinfo->ui_accounting.ha_resources[resource] -= amount; + if (cred->cr_ruidinfo != cred->cr_uidinfo) + cred->cr_uidinfo->ui_accounting.ha_resources[resource] -= amount; for (pr = cred->cr_prison; pr != NULL; pr = pr->pr_parent) pr->pr_accounting.ha_resources[resource] -= amount; - for (i = 0; i < cred->cr_ngroups; i++) + /* + * XXX: Slow. + */ + for (i = 0; i < cred->cr_ngroups; i++) { + /* + * Make sure we don't account a group more than once if it appears + * in cr_groups[] more than once. + */ + for (j = 0; j < i; j++) { + if (cred->cr_groups[i] == cred->cr_groups[j]) + goto skip_group; + } cred->cr_gidinfos[i]->gi_accounting.ha_resources[resource] -= amount; - +skip_group: + continue; + } #ifdef INVARIANTS hrl_assert_proc(p); #endif + mtx_unlock(&hrl_lock); } /* @@ -263,7 +419,7 @@ node = RB_FIND(hrl_tree, &hrls, &searched); if (node != NULL) { node = RB_REMOVE(hrl_tree, &hrls, node); - KASSERT(node != NULL, ("node removal failed")); + KASSERT(node != NULL, ("hrl_adjust: node removal failed")); } mtx_unlock(&hrl_lock); if (node != NULL) @@ -292,16 +448,18 @@ { int i; + mtx_lock(&hrl_lock); for (i = 0; i < HRL_RESOURCE_MAX; i++) { #ifdef notyet KASSERT(dest->ha_resources[i] >= 0, ("resource usage propagation meltdown")); + KASSERT(src->ha_resources[i] >= 0, ("resource usage propagation meltdown")); #endif dest->ha_resources[i] += src->ha_resources[i]; #ifdef notyet - KASSERT(src->ha_resources[i] >= 0, ("resource usage propagation meltdown")); KASSERT(dest->ha_resources[i] >= 0, ("resource usage propagation meltdown")); #endif } + mtx_unlock(&hrl_lock); } void @@ -309,16 +467,19 @@ { int i; + mtx_lock(&hrl_lock); for (i = 0; i < HRL_RESOURCE_MAX; i++) { #ifdef notyet KASSERT(dest->ha_resources[i] >= 0, ("resource usage propagation meltdown")); + KASSERT(src->ha_resources[i] >= 0, ("resource usage propagation meltdown")); + KASSERT(src->ha_resources[i] <= dest->ha_resources[i], ("resource usage propagation meltdown")); #endif dest->ha_resources[i] -= src->ha_resources[i]; #ifdef notyet - KASSERT(src->ha_resources[i] >= 0, ("resource usage propagation meltdown")); KASSERT(dest->ha_resources[i] >= 0, ("resource usage propagation meltdown")); #endif } + mtx_unlock(&hrl_lock); } /* @@ -526,7 +687,7 @@ continue; node = RB_REMOVE(hrl_tree, &hrls, node); - KASSERT(node != NULL, ("node removal failed")); + KASSERT(node != NULL, ("hrl_proc_exit: node removal failed")); mtx_unlock(&hrl_lock); uma_zfree(hrl_zone, node); @@ -541,6 +702,6 @@ hrl_zone = uma_zcreate("hrl", sizeof(struct hrl_node), NULL, NULL, NULL, NULL, UMA_ALIGN_PTR, UMA_ZONE_NOFREE); - mtx_init(&hrl_lock, "hrl lock", NULL, MTX_DEF); + mtx_init(&hrl_lock, "hrl lock", NULL, MTX_RECURSE); /* XXX: Make it non-recurseable later. */ EVENTHANDLER_REGISTER(process_exit, hrl_proc_exit, NULL, EVENTHANDLER_PRI_ANY); } ==== //depot/projects/soc2009/trasz_limits/sys/sys/hrl.h#10 (text+ko) ==== @@ -120,6 +120,8 @@ void hrl_adjust(int subject, id_t subject_id, int per, int resource, int action, int64_t amount); void hrl_acc_add(struct hrl_acc *dest, const struct hrl_acc *src); void hrl_acc_subtract(struct hrl_acc *dest, const struct hrl_acc *src); +void hrl_proc_exiting(struct proc *p); +void hrl_proc_fork(struct proc *parent, struct proc *child); #else /* !_KERNEL */ ==== //depot/projects/soc2009/trasz_limits/sys/sys/ucred.h#5 (text+ko) ==== @@ -84,7 +84,9 @@ #ifdef _KERNEL struct thread; +struct proc; +void change_cred(struct proc *p, struct ucred *newcred); void change_egid(struct ucred *newcred, struct gidinfo *egip); void change_euid(struct ucred *newcred, struct uidinfo *euip); void change_rgid(struct ucred *newcred, gid_t rgid);