On 8/18/24 22:14, Waiman Long wrote:
On 8/16/24 04:27, Chen Ridong wrote:
When enable a remote partition, I found that:
cd /sys/fs/cgroup/
mkdir test
mkdir test/test1
echo +cpuset > cgroup.subtree_control
echo +cpuset > test/cgroup.subtree_control
echo 3 > test/test1/cpuset.cpus
echo root > test/test1/cpuset.cpus.partition
cat test/test1/cpuset.cpus.partition
root invalid (Parent is not a partition root)
The parent of a remote partition could not be a root. This is due to the
emtpy effective_xcpus. It would be better to prompt the message "invalid
cpu list in cpuset.cpus.exclusive".
Signed-off-by: Chen Ridong <chenridong@xxxxxxxxxx>
---
kernel/cgroup/cpuset.c | 42 +++++++++++++++++++++++-------------------
1 file changed, 23 insertions(+), 19 deletions(-)
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index e34fd6108b06..fdd5346616d3 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -80,6 +80,7 @@ enum prs_errcode {
PERR_HOTPLUG,
PERR_CPUSEMPTY,
PERR_HKEEPING,
+ PERR_PMT,
One more thing, the "PMT" acronym for the error code is hard to decode.
I will suggest you either use the "PERMISSION" or "ACCESS" like the
EACCESS errno.
Cheers,
Longman
};
static const char * const perr_strings[] = {
@@ -91,6 +92,7 @@ static const char * const perr_strings[] = {
[PERR_HOTPLUG] = "No cpu available due to hotplug",
[PERR_CPUSEMPTY] = "cpuset.cpus and cpuset.cpus.exclusive are
empty",
[PERR_HKEEPING] = "partition config conflicts with
housekeeping setup",
+ [PERR_PMT] = "Enable partition not permitted",
};
struct cpuset {
@@ -1669,7 +1671,7 @@ static int remote_partition_enable(struct
cpuset *cs, int new_prs,
* The user must have sysadmin privilege.
*/
if (!capable(CAP_SYS_ADMIN))
- return 0;
+ return PERR_PMT;
/*
* The requested exclusive_cpus must not be allocated to other
@@ -1683,7 +1685,7 @@ static int remote_partition_enable(struct
cpuset *cs, int new_prs,
if (cpumask_empty(tmp->new_cpus) ||
cpumask_intersects(tmp->new_cpus, subpartitions_cpus) ||
cpumask_subset(top_cpuset.effective_cpus, tmp->new_cpus))
- return 0;
+ return PERR_INVCPUS;
spin_lock_irq(&callback_lock);
isolcpus_updated = partition_xcpus_add(new_prs, NULL,
tmp->new_cpus);
@@ -1698,7 +1700,7 @@ static int remote_partition_enable(struct
cpuset *cs, int new_prs,
*/
update_tasks_cpumask(&top_cpuset, tmp->new_cpus);
update_sibling_cpumasks(&top_cpuset, NULL, tmp);
- return 1;
+ return 0;
}
Since you are changing the meaning of the function returned value, you
should also update the return value comment as well.
/*
@@ -3151,24 +3153,26 @@ static int update_prstate(struct cpuset *cs,
int new_prs)
goto out;
if (!old_prs) {
- enum partition_cmd cmd = (new_prs == PRS_ROOT)
- ? partcmd_enable : partcmd_enablei;
-
/*
- * cpus_allowed and exclusive_cpus cannot be both empty.
- */
- if (xcpus_empty(cs)) {
- err = PERR_CPUSEMPTY;
- goto out;
- }
+ * If parent is valid partition, enable local partiion.
+ * Otherwise, enable a remote partition.
+ */
+ if (is_partition_valid(parent)) {
+ enum partition_cmd cmd = (new_prs == PRS_ROOT)
+ ? partcmd_enable : partcmd_enablei;
- err = update_parent_effective_cpumask(cs, cmd, NULL,
&tmpmask);
- /*
- * If an attempt to become local partition root fails,
- * try to become a remote partition root instead.
- */
- if (err && remote_partition_enable(cs, new_prs, &tmpmask))
- err = 0;
+ /*
+ * cpus_allowed and exclusive_cpus cannot be both empty.
+ */
+ if (xcpus_empty(cs)) {
+ err = PERR_CPUSEMPTY;
+ goto out;
+ }
The xcpus_empty() check should be done for both local and remote
partition.
Cheers,
Longman
+
+ err = update_parent_effective_cpumask(cs, cmd, NULL,
&tmpmask);
+ } else {
+ err = remote_partition_enable(cs, new_prs, &tmpmask);
+ }
} else if (old_prs && new_prs) {
/*
* A change in load balance state only, no change in cpumasks.
