Messages in this thread |  | | Date | Tue, 14 Dec 2021 22:24:22 -0500 | Subject | Re: [PATCH v9 2/7] cgroup/cpuset: Allow no-task partition to have empty cpuset.cpus.effective | From | Waiman Long <> |
| |
On 12/13/21 15:45, Tejun Heo wrote: > On Sun, Dec 05, 2021 at 01:32:15PM -0500, Waiman Long wrote: >> adding = deleting = false; >> old_prs = new_prs = cpuset->partition_root_state; >> if (cmd == partcmd_enable) { >> + /* >> + * Enabling partition root is not allowed if not all the CPUs >> + * can be granted from parent's effective_cpus. >> + */ >> + if (!cpumask_subset(cpuset->cpus_allowed, parent->effective_cpus)) >> + return -EINVAL; >> + >> + /* >> + * A parent can be left with no CPU as long as there is no >> + * task directly associated with the parent partition. For >> + * such a parent, no new task can be moved into it. >> + */ >> + if (partition_is_populated(parent, cpuset) && >> + cpumask_equal(cpuset->cpus_allowed, parent->effective_cpus)) >> + return -EINVAL; > So, given that this only happens with threaded domains, can we just not > allow partitions within threaded domains? The combination doesn't make whole > lot of sense to me anyway. AFAICS, there are code in cpuset.c that disallows the an non-child node to hold tasks, but the check doesn't cover all the possible cases. I remembered that I was able to create such a scenario without using threaded domains. That is why I put in this conditional check. It has nothing to do with the use of threaded domains. >> + /* >> + * On default hierarchy, task cannot be moved to a cpuset with empty >> + * effective cpus. >> + */ >> + if (is_in_v2_mode() && cpumask_empty(cs->effective_cpus)) >> + goto out_unlock; > And then we can avoid this extra restriction too, right?
This check is supposed to prevent a task to be moved to a leaf cpuset partition with just offlined cpus and hence no effective cpu. A possible alternative is to force the partition to become invalid, but I think not allowing the move is easier until one or more offlined cpus are onlined.
Cheers, Longman
|  |