Date: Sun, 9 Aug 2009 12:00:48 GMT From: Edward Tomasz Napierala <trasz@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 167128 for review Message-ID: <200908091200.n79C0mtV095922@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=167128 Change 167128 by trasz@trasz_anger on 2009/08/09 12:00:44 Rewrite rule enforcement to behave as originally designed. The per-cpu trick, which I think is a crucial for acceptable performance, is still not there. Affected files ... .. //depot/projects/soc2009/trasz_limits/sys/kern/kern_hrl.c#50 edit Differences ... ==== //depot/projects/soc2009/trasz_limits/sys/kern/kern_hrl.c#50 (text+ko) ==== @@ -112,8 +112,7 @@ static uma_zone_t hrl_rule_zone; static struct mtx hrl_lock; -static void hrl_compute_available(struct proc *p, int64_t (*availablep)[], - struct hrl_rule *(*rulesp)[]); +static void hrl_compute_available(struct proc *p, int64_t (*availablep)[]); static int hrl_rule_fully_specified(const struct hrl_rule *rule); static void hrl_rule_to_sbuf(struct sbuf *sb, const struct hrl_rule *rule); @@ -240,15 +239,95 @@ } /* + * Return the amount of resource that can be allocated by 'p' before + * hitting 'rule'. + */ +static int64_t +hrl_available_resource(const struct proc *p, const struct hrl_rule *rule) +{ + int resource; + int64_t available; + + mtx_assert(&hrl_lock, MA_OWNED); + + resource = rule->hr_resource; + switch (rule->hr_per) { + case HRL_SUBJECT_PROCESS: + available = rule->hr_amount - + p->p_usage.hu_resources[resource]; + break; + case HRL_SUBJECT_USER: + available = rule->hr_amount - + p->p_ucred->cr_ruidinfo->ui_usage.hu_resources[resource]; + break; + case HRL_SUBJECT_GROUP: + if (hrl_group_accounting) { + /* + * XXX + */ + available = INT64_MAX; + } + break; + case HRL_SUBJECT_LOGINCLASS: + /* + * XXX + */ + available = INT64_MAX; + break; + case HRL_SUBJECT_JAIL: + /* + * XXX + */ + available = INT64_MAX; + break; + default: + panic("hrl_compute_available: unknown per %d", + rule->hr_per); + } + + return (available); +} + +/* + * Return non-zero if allocating 'amount' by proc 'p' would exceed + * resource limit specified by 'rule'. + */ +static int +hrl_would_exceed(const struct proc *p, const struct hrl_rule *rule, + int64_t amount) +{ + int64_t available; + + mtx_assert(&hrl_lock, MA_OWNED); + + available = hrl_available_resource(p, rule); + if (available >= amount) + return (0); + + /* + * We've already exceeded that one. + */ + if (available < 0) { +#ifdef notyet + KASSERT(rule->hr_action != HRL_ACTION_DENY, + ("hrl_would_exceed: deny rule already exceeded")); +#endif + return (0); + } + + return (1); +} + +/* * Check whether the proc 'p' can allocate 'amount' of 'resource' in addition - * to what it has allocated now. Returns fake error code if the allocation - * should fail. + * to what it keeps allocated now. Returns non-zero if the allocation should + * be denied, 0 otherwise. */ static int hrl_enforce_proc(struct proc *p, int resource, uint64_t amount) { int64_t available[HRL_RESOURCE_MAX]; - struct hrl_rule *rules[HRL_RESOURCE_MAX], *rule; + struct hrl_rule *rule; struct hrl_limit *limit; struct sbuf *sb; int should_deny = 0; @@ -258,7 +337,7 @@ /* * XXX: Do this just before we start running on a CPU, not all the time. */ - hrl_compute_available(p, &available, &rules); + hrl_compute_available(p, &available); if (available[resource] >= amount) return (0); @@ -267,6 +346,10 @@ * It seems we've hit a limit. Figure out what to do. There may * be more than one matching limit; go through all of them. Denial * should be done last, after logging and sending signals. + * + * Note that it is possible to get here, and still not trigger + * any limit, because some of the resource got freed on another + * CPU after computing contents of the 'available' array. */ /* * XXX: We should sort the rules somewhat, so that 'log' and 'sig' @@ -277,14 +360,9 @@ rule = limit->hl_rule; if (rule->hr_resource != resource) continue; - if (rule->hr_amount < available[resource]); - continue; - if (rule->hr_amount > available[resource] + amount); + if (!hrl_would_exceed(p, rule, amount)) continue; - /* - * This rule should apply to us. Behave accordingly. - */ switch (rule->hr_action) { case HRL_ACTION_DENY: should_deny = 1; @@ -334,36 +412,35 @@ } /* - * Go through all the rules applicable to the process, fill first array - * with amount of resource left before hitting next limit, and the second - * with pointers to the limit to be hit. + * Go through all the rules applicable to the process, filling the array + * with amount of resource left before hitting the next limit. */ static void -hrl_compute_available(struct proc *p, int64_t (*availablep)[], - struct hrl_rule *(*rulesp)[]) +hrl_compute_available(struct proc *p, int64_t (*availablep)[]) { int i, resource; int64_t available; struct hrl_limit *limit; + struct hrl_rule *rule; mtx_assert(&hrl_lock, MA_OWNED); - for (i = 0; i < HRL_RESOURCE_MAX; i++) { + for (i = 0; i < HRL_RESOURCE_MAX; i++) (*availablep)[i] = INT64_MAX; - (*rulesp)[i] = NULL; - } LIST_FOREACH(limit, &p->p_limits, hl_next) { - resource = limit->hl_rule->hr_resource; - available = limit->hl_rule->hr_amount - - p->p_usage.hu_resources[resource]; - /* Skip limits that have been already exceeded. */ - if (available < 0) + rule = limit->hl_rule; + resource = rule->hr_resource; + available = hrl_available_resource(p, rule); + if (available < 0) { +#ifdef notyet + KASSERT(rule->hr_action != HRL_ACTION_DENY, + ("hrl_compute_available: deny rule already exceeded")); +#endif continue; - if (available < (*availablep)[resource]) { + } + if (available < (*availablep)[resource]) (*availablep)[resource] = available; - (*rulesp)[resource] = limit->hl_rule; - } } }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200908091200.n79C0mtV095922>