Date: Thu, 4 Mar 2021 20:55:48 GMT From: Kyle Evans <kevans@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: 3d4229b5decc - stable/12 - Revert "MFC kern: cpuset: properly rebase when attaching to a jail" Message-ID: <202103042055.124KtmAI008586@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/12 has been updated by kevans: URL: https://cgit.FreeBSD.org/src/commit/?id=3d4229b5decc74b5276fa0b3930ed1552c5f00f5 commit 3d4229b5decc74b5276fa0b3930ed1552c5f00f5 Author: Kyle Evans <kevans@FreeBSD.org> AuthorDate: 2021-03-04 20:13:55 +0000 Commit: Kyle Evans <kevans@FreeBSD.org> CommitDate: 2021-03-04 20:50:34 +0000 Revert "MFC kern: cpuset: properly rebase when attaching to a jail" This behavior change is too invasive to be made between minor versions, back it out in stable/12 -- it will be first introduced in 13.0. The cpuset test has been adjusted to account for the legacy behavior, with a note added as to why it's different and doesn't work if run as-is on 13.0. This reverts commit 7bb960ce6447bd535e0fbb648e4d9edbb1dc067f. --- lib/libc/tests/sys/cpuset_test.c | 11 +++- sys/kern/kern_cpuset.c | 121 +++++++-------------------------------- 2 files changed, 31 insertions(+), 101 deletions(-) diff --git a/lib/libc/tests/sys/cpuset_test.c b/lib/libc/tests/sys/cpuset_test.c index d6dd69e7e3c1..e3332540eb29 100644 --- a/lib/libc/tests/sys/cpuset_test.c +++ b/lib/libc/tests/sys/cpuset_test.c @@ -360,7 +360,16 @@ jail_attach_newbase_epi(struct jail_test_cb_params *cbp) */ ATF_REQUIRE(info->jail_cpuset != cbp->rootid); ATF_REQUIRE(info->jail_cpuset != cbp->setid); - ATF_REQUIRE(info->jail_cpuset != info->jail_child_cpuset); + + /* + * The historical behavior has been that the process will simply take on + * and mask the jail's cpuset. As of FreeBSD 13.0, this behavior will + * change so that an attaching process will rebase its cpuset onto the + * jail's if it had one distinct from its own jail's root, thus breaking + * this condition. + */ + ATF_REQUIRE(info->jail_cpuset == info->jail_child_cpuset); + ATF_REQUIRE_EQ(0, CPU_CMP(mask, &info->jail_tidmask)); } diff --git a/sys/kern/kern_cpuset.c b/sys/kern/kern_cpuset.c index 368acdb2be16..e2e45f53c0af 100644 --- a/sys/kern/kern_cpuset.c +++ b/sys/kern/kern_cpuset.c @@ -1138,63 +1138,14 @@ cpuset_setproc_setthread(struct cpuset *tdset, struct cpuset *set, domainlist); } -static int -cpuset_setproc_newbase(struct thread *td, struct cpuset *set, - struct cpuset *nroot, struct cpuset **nsetp, - struct setlist *cpusets, struct domainlist *domainlist) -{ - struct domainset ndomain; - cpuset_t nmask; - struct cpuset *pbase; - int error; - - pbase = cpuset_getbase(td->td_cpuset); - - /* Copy process mask, then further apply the new root mask. */ - CPU_COPY(&pbase->cs_mask, &nmask); - CPU_AND(&nmask, &nroot->cs_mask); - - domainset_copy(pbase->cs_domain, &ndomain); - DOMAINSET_AND(&ndomain.ds_mask, &set->cs_domain->ds_mask); - - /* Policy is too restrictive, will not work. */ - if (CPU_EMPTY(&nmask) || DOMAINSET_EMPTY(&ndomain.ds_mask)) - return (EDEADLK); - - /* - * Remove pbase from the freelist in advance, it'll be pushed to - * cpuset_ids on success. We assume here that cpuset_create() will not - * touch pbase on failure, and we just enqueue it back to the freelist - * to remain in a consistent state. - */ - pbase = LIST_FIRST(cpusets); - LIST_REMOVE(pbase, cs_link); - error = cpuset_create(&pbase, set, &nmask); - if (error != 0) { - LIST_INSERT_HEAD(cpusets, pbase, cs_link); - return (error); - } - - /* Duplicates some work from above... oh well. */ - pbase->cs_domain = domainset_shadow(set->cs_domain, &ndomain, - domainlist); - *nsetp = pbase; - return (0); -} - /* - * Handle four cases for updating an entire process. + * Handle three cases for updating an entire process. * - * 1) Set is non-null and the process is not rebasing onto a new root. This - * reparents all anonymous sets to the provided set and replaces all - * non-anonymous td_cpusets with the provided set. - * 2) Set is non-null and the process is rebasing onto a new root. This - * creates a new base set if the process previously had its own base set, - * then reparents all anonymous sets either to that set or the provided set - * if one was not created. Non-anonymous sets are similarly replaced. - * 3) Mask is non-null. This replaces or creates anonymous sets for every + * 1) Set is non-null. This reparents all anonymous sets to the provided + * set and replaces all non-anonymous td_cpusets with the provided set. + * 2) Mask is non-null. This replaces or creates anonymous sets for every * thread with the existing base as a parent. - * 4) domain is non-null. This creates anonymous sets for every thread + * 3) domain is non-null. This creates anonymous sets for every thread * and replaces the domain set. * * This is overly complicated because we can't allocate while holding a @@ -1203,15 +1154,15 @@ cpuset_setproc_newbase(struct thread *td, struct cpuset *set, */ static int cpuset_setproc(pid_t pid, struct cpuset *set, cpuset_t *mask, - struct domainset *domain, bool rebase) + struct domainset *domain) { struct setlist freelist; struct setlist droplist; struct domainlist domainlist; - struct cpuset *base, *nset, *nroot, *tdroot; + struct cpuset *nset; struct thread *td; struct proc *p; - int needed; + int threads; int nfree; int error; @@ -1227,49 +1178,21 @@ cpuset_setproc(pid_t pid, struct cpuset *set, cpuset_t *mask, nfree = 1; LIST_INIT(&droplist); nfree = 0; - base = set; - nroot = NULL; - if (set != NULL) - nroot = cpuset_getroot(set); for (;;) { error = cpuset_which(CPU_WHICH_PID, pid, &p, &td, &nset); if (error) goto out; - tdroot = cpuset_getroot(td->td_cpuset); - needed = p->p_numthreads; - if (set != NULL && rebase && tdroot != nroot) - needed++; - if (nfree >= needed) + if (nfree >= p->p_numthreads) break; + threads = p->p_numthreads; PROC_UNLOCK(p); - if (nfree < needed) { - cpuset_freelist_add(&freelist, needed - nfree); - domainset_freelist_add(&domainlist, needed - nfree); - nfree = needed; + if (nfree < threads) { + cpuset_freelist_add(&freelist, threads - nfree); + domainset_freelist_add(&domainlist, threads - nfree); + nfree = threads; } } PROC_LOCK_ASSERT(p, MA_OWNED); - - /* - * If we're changing roots and the root set is what has been specified - * as the parent, then we'll check if the process was previously using - * the root set and, if it wasn't, create a new base with the process's - * mask applied to it. - */ - if (set != NULL && rebase && nroot != tdroot) { - cpusetid_t base_id, root_id; - - root_id = td->td_ucred->cr_prison->pr_cpuset->cs_id; - base_id = cpuset_getbase(td->td_cpuset)->cs_id; - - if (base_id != root_id) { - error = cpuset_setproc_newbase(td, set, nroot, &base, - &freelist, &domainlist); - if (error != 0) - goto unlock_out; - } - } - /* * Now that the appropriate locks are held and we have enough cpusets, * make sure the operation will succeed before applying changes. The @@ -1280,7 +1203,7 @@ cpuset_setproc(pid_t pid, struct cpuset *set, cpuset_t *mask, thread_lock(td); if (set != NULL) error = cpuset_setproc_test_setthread(td->td_cpuset, - base); + set); else error = cpuset_setproc_test_maskthread(td->td_cpuset, mask, domain); @@ -1296,7 +1219,7 @@ cpuset_setproc(pid_t pid, struct cpuset *set, cpuset_t *mask, FOREACH_THREAD_IN_PROC(p, td) { thread_lock(td); if (set != NULL) - error = cpuset_setproc_setthread(td->td_cpuset, base, + error = cpuset_setproc_setthread(td->td_cpuset, set, &nset, &freelist, &domainlist); else error = cpuset_setproc_maskthread(td->td_cpuset, mask, @@ -1311,8 +1234,6 @@ cpuset_setproc(pid_t pid, struct cpuset *set, cpuset_t *mask, unlock_out: PROC_UNLOCK(p); out: - if (base != NULL && base != set) - cpuset_rel(base); while ((nset = LIST_FIRST(&droplist)) != NULL) cpuset_rel_complete(nset); cpuset_freelist_free(&freelist); @@ -1697,7 +1618,7 @@ cpuset_setproc_update_set(struct proc *p, struct cpuset *set) KASSERT(set != NULL, ("[%s:%d] invalid set", __func__, __LINE__)); cpuset_ref(set); - error = cpuset_setproc(p->p_pid, set, NULL, NULL, true); + error = cpuset_setproc(p->p_pid, set, NULL, NULL); if (error) return (error); cpuset_rel(set); @@ -1747,7 +1668,7 @@ sys_cpuset(struct thread *td, struct cpuset_args *uap) return (error); error = copyout(&set->cs_id, uap->setid, sizeof(set->cs_id)); if (error == 0) - error = cpuset_setproc(-1, set, NULL, NULL, false); + error = cpuset_setproc(-1, set, NULL, NULL); cpuset_rel(set); return (error); } @@ -1781,7 +1702,7 @@ kern_cpuset_setid(struct thread *td, cpuwhich_t which, set = cpuset_lookup(setid, td); if (set == NULL) return (ESRCH); - error = cpuset_setproc(id, set, NULL, NULL, false); + error = cpuset_setproc(id, set, NULL, NULL); cpuset_rel(set); return (error); } @@ -2060,7 +1981,7 @@ kern_cpuset_setaffinity(struct thread *td, cpulevel_t level, cpuwhich_t which, error = cpuset_setthread(id, mask); break; case CPU_WHICH_PID: - error = cpuset_setproc(id, NULL, mask, NULL, false); + error = cpuset_setproc(id, NULL, mask, NULL); break; case CPU_WHICH_CPUSET: case CPU_WHICH_JAIL: @@ -2349,7 +2270,7 @@ kern_cpuset_setdomain(struct thread *td, cpulevel_t level, cpuwhich_t which, error = _cpuset_setthread(id, NULL, &domain); break; case CPU_WHICH_PID: - error = cpuset_setproc(id, NULL, NULL, &domain, false); + error = cpuset_setproc(id, NULL, NULL, &domain); break; case CPU_WHICH_CPUSET: case CPU_WHICH_JAIL:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202103042055.124KtmAI008586>