Date: Wed, 14 Jul 2010 19:48:48 GMT From: Edward Tomasz Napierala <trasz@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 180961 for review Message-ID: <201007141948.o6EJmmc5093168@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://p4web.freebsd.org/@@180961?ac=10 Change 180961 by trasz@trasz_victim on 2010/07/14 19:48:20 Get rid of 'struct gidinfo'. Per-group resource limits are not going to happen; it would be bad performance-wise and would complicate the code (right now each container can have at most three containing containers, so we can store parents in a simple, fixed size array; this wouldn't work with per-group limits). Besides, in most cases you want to use per-group or per-loginclass limits anyway. Affected files ... .. //depot/projects/soc2009/trasz_limits/sys/kern/kern_hrl.c#87 edit .. //depot/projects/soc2009/trasz_limits/sys/kern/kern_proc.c#14 edit .. //depot/projects/soc2009/trasz_limits/sys/kern/kern_resource.c#37 edit .. //depot/projects/soc2009/trasz_limits/sys/sys/hrl.h#47 edit .. //depot/projects/soc2009/trasz_limits/sys/sys/resourcevar.h#20 edit Differences ... ==== //depot/projects/soc2009/trasz_limits/sys/kern/kern_hrl.c#87 (text+ko) ==== @@ -85,7 +85,6 @@ static struct dict subjectnames[] = { { "process", HRL_SUBJECT_TYPE_PROCESS }, { "user", HRL_SUBJECT_TYPE_USER }, - { "group", HRL_SUBJECT_TYPE_GROUP }, { "loginclass", HRL_SUBJECT_TYPE_LOGINCLASS }, { "jail", HRL_SUBJECT_TYPE_JAIL }, { NULL, -1 }}; @@ -359,11 +358,6 @@ filter->hr_subject.hs_uip) return (0); break; - case HRL_SUBJECT_TYPE_GROUP: - if (filter->hr_subject.hs_gip != - filter->hr_subject.hs_gip) - return (0); - break; case HRL_SUBJECT_TYPE_LOGINCLASS: if (filter->hr_subject.hs_loginclass != filter->hr_subject.hs_loginclass) @@ -526,10 +520,6 @@ if (rule->hr_subject.hs_uip != NULL) uihold(rule->hr_subject.hs_uip); break; - case HRL_SUBJECT_TYPE_GROUP: - if (rule->hr_subject.hs_gip != NULL) - gihold(rule->hr_subject.hs_gip); - break; case HRL_SUBJECT_TYPE_LOGINCLASS: if (rule->hr_subject.hs_loginclass != NULL) loginclass_acquire(rule->hr_subject.hs_loginclass); @@ -556,10 +546,6 @@ if (rule->hr_subject.hs_uip != NULL) uifree(rule->hr_subject.hs_uip); break; - case HRL_SUBJECT_TYPE_GROUP: - if (rule->hr_subject.hs_gip != NULL) - gifree(rule->hr_subject.hs_gip); - break; case HRL_SUBJECT_TYPE_LOGINCLASS: if (rule->hr_subject.hs_loginclass != NULL) loginclass_release(rule->hr_subject.hs_loginclass); @@ -588,7 +574,6 @@ rule->hr_subject_type = HRL_SUBJECT_TYPE_UNDEFINED; rule->hr_subject.hs_proc = NULL; rule->hr_subject.hs_uip = NULL; - rule->hr_subject.hs_gip = NULL; rule->hr_subject.hs_loginclass = NULL; rule->hr_subject.hs_prison = NULL; rule->hr_per = HRL_SUBJECT_TYPE_UNDEFINED; @@ -611,7 +596,6 @@ copy->hr_subject_type = rule->hr_subject_type; copy->hr_subject.hs_proc = rule->hr_subject.hs_proc; copy->hr_subject.hs_uip = rule->hr_subject.hs_uip; - copy->hr_subject.hs_gip = rule->hr_subject.hs_gip; copy->hr_subject.hs_loginclass = rule->hr_subject.hs_loginclass; copy->hr_subject.hs_prison = rule->hr_subject.hs_prison; copy->hr_per = rule->hr_per; @@ -660,10 +644,6 @@ if (rule->hr_subject.hs_uip == NULL) return (0); break; - case HRL_SUBJECT_TYPE_GROUP: - if (rule->hr_subject.hs_gip == NULL) - return (0); - break; case HRL_SUBJECT_TYPE_LOGINCLASS: if (rule->hr_subject.hs_loginclass == NULL) return (0); @@ -717,7 +697,6 @@ if (subject_idstr == NULL || subject_idstr[0] == '\0') { rule->hr_subject.hs_proc = NULL; rule->hr_subject.hs_uip = NULL; - rule->hr_subject.hs_gip = NULL; rule->hr_subject.hs_loginclass = NULL; rule->hr_subject.hs_prison = NULL; } else { @@ -746,9 +725,6 @@ case HRL_SUBJECT_TYPE_USER: rule->hr_subject.hs_uip = uifind(id); break; - case HRL_SUBJECT_TYPE_GROUP: - rule->hr_subject.hs_gip = gifind(id); - break; case HRL_SUBJECT_TYPE_LOGINCLASS: rule->hr_subject.hs_loginclass = loginclass_find(subject_idstr); break; @@ -818,16 +794,11 @@ struct proc *p; struct ucred *cred; struct uidinfo *uip; - struct gidinfo *gip; struct prison *pr; struct loginclass *lc; KASSERT(hrl_rule_fully_specified(rule), ("rule not fully specified")); - if (rule->hr_subject_type == HRL_SUBJECT_TYPE_GROUP || - rule->hr_per == HRL_SUBJECT_TYPE_GROUP) - return (EOPNOTSUPP); - if (rule->hr_action == HRL_ACTION_DELAY) return (EOPNOTSUPP); @@ -855,12 +826,6 @@ hrl_container_add_rule(&uip->ui_container, rule); break; - case HRL_SUBJECT_TYPE_GROUP: - gip = rule->hr_subject.hs_gip; - KASSERT(gip != NULL, ("hrl_rule_add: NULL gip")); - hrl_container_add_rule(&gip->gi_container, rule); - break; - case HRL_SUBJECT_TYPE_LOGINCLASS: lc = rule->hr_subject.hs_loginclass; KASSERT(lc != NULL, ("hrl_rule_add: NULL loginclass")); @@ -891,10 +856,6 @@ cred->cr_ruidinfo == rule->hr_subject.hs_uip) break; continue; - case HRL_SUBJECT_TYPE_GROUP: - if (groupmember(rule->hr_subject.hs_gip->gi_gid, cred)) - break; - continue; case HRL_SUBJECT_TYPE_LOGINCLASS: if (cred->cr_loginclass == rule->hr_subject.hs_loginclass) break; @@ -948,9 +909,6 @@ error = ui_container_foreach(hrl_rule_remove_callback, filter, (void *)&found); KASSERT(error == 0, ("ui_container_foreach failed")); - error = gi_container_foreach(hrl_rule_remove_callback, filter, - (void *)&found); - KASSERT(error == 0, ("gi_container_foreach failed")); sx_assert(&allproc_lock, SA_LOCKED); FOREACH_PROC_IN_SYSTEM(p) { @@ -986,12 +944,6 @@ else sbuf_printf(sb, "%d:", rule->hr_subject.hs_uip->ui_uid); break; - case HRL_SUBJECT_TYPE_GROUP: - if (rule->hr_subject.hs_gip == NULL) - sbuf_printf(sb, ":"); - else - sbuf_printf(sb, "%d:", rule->hr_subject.hs_gip->gi_gid); - break; case HRL_SUBJECT_TYPE_LOGINCLASS: if (rule->hr_subject.hs_loginclass == NULL) sbuf_printf(sb, ":"); @@ -1088,7 +1040,6 @@ struct sbuf *outputsbuf = NULL; struct proc *p; struct uidinfo *uip; - struct gidinfo *gip; struct loginclass *lc; struct prison *pr; @@ -1121,14 +1072,6 @@ } outputsbuf = hrl_container_to_sbuf(&uip->ui_container); break; - case HRL_SUBJECT_TYPE_GROUP: - gip = filter->hr_subject.hs_gip; - if (gip == NULL) { - error = EINVAL; - goto out; - } - outputsbuf = hrl_container_to_sbuf(&gip->gi_container); - break; case HRL_SUBJECT_TYPE_LOGINCLASS: lc = filter->hr_subject.hs_loginclass; if (lc == NULL) { @@ -1227,7 +1170,6 @@ 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(&hrl_lock); if (sbuf_overflowed(sb)) { sbuf_delete(sb); ==== //depot/projects/soc2009/trasz_limits/sys/kern/kern_proc.c#14 (text+ko) ==== @@ -173,7 +173,6 @@ proc_ctor, proc_dtor, proc_init, proc_fini, UMA_ALIGN_PTR, UMA_ZONE_NOFREE); uihashinit(); - gihashinit(); } /* ==== //depot/projects/soc2009/trasz_limits/sys/kern/kern_resource.c#37 (text+ko) ==== @@ -73,18 +73,12 @@ static struct rwlock uihashtbl_lock; static LIST_HEAD(uihashhead, uidinfo) *uihashtbl; static u_long uihash; /* size of hash table - 1 */ -static MALLOC_DEFINE(M_GIDINFO, "gidinfo", "gidinfo structures"); -#define GIHASH(gid) (&gihashtbl[(gid) & gihash]) -static struct rwlock gihashtbl_lock; -static LIST_HEAD(gihashhead, gidinfo) *gihashtbl; -static u_long gihash; /* size of hash table - 1 */ static void calcru1(struct proc *p, struct rusage_ext *ruxp, struct timeval *up, struct timeval *sp); static int donice(struct thread *td, struct proc *chgp, int n); static struct uidinfo *uilookup(uid_t uid); static void ruxagg_locked(struct rusage_ext *rux, struct thread *td); -static struct gidinfo *gilookup(gid_t gid); /* * Resource controls and accounting. @@ -1446,158 +1440,6 @@ #endif /* - * Find the gidinfo structure for a gid. This structure is used to - * track the total resource consumption (process count, socket buffer - * size, etc.) for the gid and impose limits. - */ -void -gihashinit() -{ - - gihashtbl = hashinit(maxproc / 16, M_GIDINFO, &gihash); - rw_init(&gihashtbl_lock, "gidinfo hash"); -} - -/* - * Look up a gidinfo struct for the parameter gid. - * gihashtbl_lock must be locked. - */ -static struct gidinfo * -gilookup(gid) - gid_t gid; -{ - struct gihashhead *gipp; - struct gidinfo *gip; - - rw_assert(&gihashtbl_lock, RA_LOCKED); - gipp = GIHASH(gid); - LIST_FOREACH(gip, gipp, gi_hash) - if (gip->gi_gid == gid) - break; - - return (gip); -} - -/* - * Find or allocate a struct gidinfo for a particular gid. - * Increase refcount on gidinfo struct returned. - * gifree() should be called on a struct gidinfo when released. - */ -struct gidinfo * -gifind(gid) - gid_t gid; -{ - struct gidinfo *old_gip, *gip; - - rw_rlock(&gihashtbl_lock); - gip = gilookup(gid); - if (gip == NULL) { - rw_runlock(&gihashtbl_lock); - gip = malloc(sizeof(*gip), M_GIDINFO, M_WAITOK | M_ZERO); - rw_wlock(&gihashtbl_lock); - /* - * There's a chance someone created our gidinfo while we - * were in malloc and not holding the lock, so we have to - * make sure we don't insert a duplicate gidinfo. - */ - if ((old_gip = gilookup(gid)) != NULL) { - /* Someone else beat us to it. */ - free(gip, M_GIDINFO); - gip = old_gip; - } else { - refcount_init(&gip->gi_ref, 0); - gip->gi_gid = gid; - LIST_INSERT_HEAD(GIHASH(gid), gip, gi_hash); - } - } - gihold(gip); - rw_unlock(&gihashtbl_lock); - return (gip); -} - -/* - * Place another refcount on a gidinfo struct. - */ -void -gihold(gip) - struct gidinfo *gip; -{ - - refcount_acquire(&gip->gi_ref); -} - -/*- - * Since gidinfo structs have a long lifetime, we use an - * opportunistic refcounting scheme to avoid locking the lookup hash - * for each release. - * - * If the refcount hits 0, we need to free the structure, - * which means we need to lock the hash. - * Optimal case: - * After locking the struct and lowering the refcount, if we find - * that we don't need to free, simply unlock and return. - * Suboptimal case: - * If refcount lowering results in need to free, bump the count - * back up, lose the lock and acquire the locks in the proper - * order to try again. - */ -void -gifree(gip) - struct gidinfo *gip; -{ - int old; - - /* Prepare for optimal case. */ - old = gip->gi_ref; - if (old > 1 && atomic_cmpset_int(&gip->gi_ref, old, old - 1)) - return; - - /* Prepare for suboptimal case. */ - rw_wlock(&gihashtbl_lock); - if (refcount_release(&gip->gi_ref)) { -#ifdef HRL - container_destroy(&gip->gi_container); -#endif - LIST_REMOVE(gip, gi_hash); - rw_wunlock(&gihashtbl_lock); - free(gip, M_GIDINFO); - return; - } - /* - * Someone added a reference between atomic_cmpset_int() and - * rw_wlock(&gihashtbl_lock). - */ - rw_wunlock(&gihashtbl_lock); -} - -#ifdef HRL -int -gi_container_foreach(int (*callback)(struct container *container, - const struct hrl_rule *filter, void *arg3), - const struct hrl_rule *filter, void *arg3) -{ - int error; - struct gidinfo *gip, *nextgip; - struct gihashhead *gih; - - rw_rlock(&gihashtbl_lock); - for (gih = &gihashtbl[gihash]; gih >= gihashtbl; gih--) { - for (gip = LIST_FIRST(gih); gip; gip = nextgip) { - nextgip = LIST_NEXT(gip, gi_hash); - error = (callback)(&gip->gi_container, filter, arg3); - if (error) { - rw_runlock(&gihashtbl_lock); - return (error); - } - } - } - rw_runlock(&gihashtbl_lock); - - return (0); -} -#endif - -/* * Change the count associated with number of processes * a given user is using. When 'max' is 0, don't enforce a limit */ ==== //depot/projects/soc2009/trasz_limits/sys/sys/hrl.h#47 (text+ko) ==== @@ -35,7 +35,6 @@ struct proc; struct uidinfo; -struct gidinfo; struct loginclass; struct prison; struct ucred; @@ -71,7 +70,6 @@ #endif struct proc *hs_proc; struct uidinfo *hs_uip; - struct gidinfo *hs_gip; struct loginclass *hs_loginclass; struct prison *hs_prison; } hr_subject; @@ -85,7 +83,6 @@ #define HRL_SUBJECT_TYPE_UNDEFINED -1 #define HRL_SUBJECT_TYPE_PROCESS 0x0000 #define HRL_SUBJECT_TYPE_USER 0x0001 -#define HRL_SUBJECT_TYPE_GROUP 0x0002 #define HRL_SUBJECT_TYPE_LOGINCLASS 0x0003 #define HRL_SUBJECT_TYPE_JAIL 0x0004 #define HRL_SUBJECT_TYPE_MAX HRL_SUBJECT_TYPE_JAIL ==== //depot/projects/soc2009/trasz_limits/sys/sys/resourcevar.h#20 (text+ko) ==== @@ -104,21 +104,6 @@ #define UIDINFO_VMSIZE_LOCK(ui) mtx_lock(&((ui)->ui_vmsize_mtx)) #define UIDINFO_VMSIZE_UNLOCK(ui) mtx_unlock(&((ui)->ui_vmsize_mtx)) -/* - * Per gid resource consumption - * - * Locking guide: - * (a) Constant from inception - * (b) Lockless, updated using atomics - * (c) Locked by global uihashtbl_mtx - */ -struct gidinfo { - LIST_ENTRY(gidinfo) gi_hash; /* (c) hash chain of gidinfos */ - gid_t gi_gid; /* (a) gid */ - u_int gi_ref; /* (b) reference count */ - struct container gi_container; /* (*) resource usage accounting */ -}; - struct proc; struct rusage_ext; struct thread; @@ -159,14 +144,6 @@ int ui_container_foreach(int (*callback)(struct container *container, const struct hrl_rule *filter, void *arg3), const struct hrl_rule *filter, void *arg3); -struct gidinfo - *gifind(gid_t gid); -void gifree(struct gidinfo *gip); -void gihashinit(void); -void gihold(struct gidinfo *gip); -int gi_container_foreach(int (*callback)(struct container *container, - const struct hrl_rule *filter, void *arg3), - const struct hrl_rule *filter, void *arg3); #endif /* _KERNEL */ #endif /* !_SYS_RESOURCEVAR_H_ */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201007141948.o6EJmmc5093168>