git: 3d4229b5decc - stable/12 - Revert "MFC kern: cpuset: properly rebase when attaching to a jail"
Kyle Evans
kevans at FreeBSD.org
Thu Mar 4 20:55:48 UTC 2021
The branch stable/12 has been updated by kevans:
URL: https://cgit.FreeBSD.org/src/commit/?id=3d4229b5decc74b5276fa0b3930ed1552c5f00f5
commit 3d4229b5decc74b5276fa0b3930ed1552c5f00f5
Author: Kyle Evans <kevans at FreeBSD.org>
AuthorDate: 2021-03-04 20:13:55 +0000
Commit: Kyle Evans <kevans at 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:
More information about the dev-commits-src-all
mailing list