From: Chen Ridong <chenridong@huawei.com>
Refactor the partition_cpus_change function to handle both regular CPU
set updates and exclusive CPU modifications, either of which may trigger
partition state changes. This generalized function will also be utilized
for exclusive CPU updates in subsequent patches.
Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
kernel/cgroup/cpuset.c | 59 ++++++++++++++++++++++++++----------------
1 file changed, 36 insertions(+), 23 deletions(-)
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 75ad18ab40ae..e3eb87a33b12 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -2447,6 +2447,41 @@ static int acpus_validate_change(struct cpuset *cs, struct cpuset *trialcs,
return retval;
}
+/**
+ * partition_cpus_change - Handle partition state changes due to CPU mask updates
+ * @cs: The target cpuset being modified
+ * @trialcs: The trial cpuset containing proposed configuration changes
+ * @tmp: Temporary masks for intermediate calculations
+ *
+ * This function handles partition state transitions triggered by CPU mask changes.
+ * CPU modifications may cause a partition to be disabled or require state updates.
+ */
+static void partition_cpus_change(struct cpuset *cs, struct cpuset *trialcs,
+ struct tmpmasks *tmp)
+{
+ if (cs_is_member(cs))
+ return;
+
+ invalidate_cs_partition(trialcs);
+ if (trialcs->prs_err)
+ cs->prs_err = trialcs->prs_err;
+
+ if (is_remote_partition(cs)) {
+ if (trialcs->prs_err)
+ remote_partition_disable(cs, tmp);
+ else
+ remote_cpus_update(cs, trialcs->exclusive_cpus,
+ trialcs->effective_xcpus, tmp);
+ } else {
+ if (trialcs->prs_err)
+ update_parent_effective_cpumask(cs, partcmd_invalidate,
+ NULL, tmp);
+ else
+ update_parent_effective_cpumask(cs, partcmd_update,
+ trialcs->effective_xcpus, tmp);
+ }
+}
+
/**
* update_cpumask - update the cpus_allowed mask of a cpuset and all tasks in it
* @cs: the cpuset to consider
@@ -2483,29 +2518,7 @@ static int update_cpumask(struct cpuset *cs, struct cpuset *trialcs,
*/
force = !cpumask_equal(cs->effective_xcpus, trialcs->effective_xcpus);
- invalidate_cs_partition(trialcs);
- if (trialcs->prs_err)
- cs->prs_err = trialcs->prs_err;
-
- if (is_partition_valid(cs) ||
- (is_partition_invalid(cs) && !trialcs->prs_err)) {
- struct cpumask *xcpus = trialcs->effective_xcpus;
-
- if (cpumask_empty(xcpus) && is_partition_invalid(cs))
- xcpus = trialcs->cpus_allowed;
-
- /*
- * Call remote_cpus_update() to handle valid remote partition
- */
- if (is_remote_partition(cs))
- remote_cpus_update(cs, NULL, xcpus, &tmp);
- else if (trialcs->prs_err)
- update_parent_effective_cpumask(cs, partcmd_invalidate,
- NULL, &tmp);
- else
- update_parent_effective_cpumask(cs, partcmd_update,
- xcpus, &tmp);
- }
+ partition_cpus_change(cs, trialcs, &tmp);
spin_lock_irq(&callback_lock);
cpumask_copy(cs->cpus_allowed, trialcs->cpus_allowed);
--
2.34.1
On 8/28/25 8:56 AM, Chen Ridong wrote: > From: Chen Ridong <chenridong@huawei.com> > > Refactor the partition_cpus_change function to handle both regular CPU > set updates and exclusive CPU modifications, either of which may trigger > partition state changes. This generalized function will also be utilized > for exclusive CPU updates in subsequent patches. > > Signed-off-by: Chen Ridong <chenridong@huawei.com> > --- > kernel/cgroup/cpuset.c | 59 ++++++++++++++++++++++++++---------------- > 1 file changed, 36 insertions(+), 23 deletions(-) > > diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c > index 75ad18ab40ae..e3eb87a33b12 100644 > --- a/kernel/cgroup/cpuset.c > +++ b/kernel/cgroup/cpuset.c > @@ -2447,6 +2447,41 @@ static int acpus_validate_change(struct cpuset *cs, struct cpuset *trialcs, > return retval; > } > > +/** > + * partition_cpus_change - Handle partition state changes due to CPU mask updates > + * @cs: The target cpuset being modified > + * @trialcs: The trial cpuset containing proposed configuration changes > + * @tmp: Temporary masks for intermediate calculations > + * > + * This function handles partition state transitions triggered by CPU mask changes. > + * CPU modifications may cause a partition to be disabled or require state updates. > + */ > +static void partition_cpus_change(struct cpuset *cs, struct cpuset *trialcs, > + struct tmpmasks *tmp) > +{ > + if (cs_is_member(cs)) > + return; > + > + invalidate_cs_partition(trialcs); > + if (trialcs->prs_err) > + cs->prs_err = trialcs->prs_err; > + > + if (is_remote_partition(cs)) { > + if (trialcs->prs_err) > + remote_partition_disable(cs, tmp); > + else > + remote_cpus_update(cs, trialcs->exclusive_cpus, > + trialcs->effective_xcpus, tmp); > + } else { > + if (trialcs->prs_err) > + update_parent_effective_cpumask(cs, partcmd_invalidate, > + NULL, tmp); > + else > + update_parent_effective_cpumask(cs, partcmd_update, > + trialcs->effective_xcpus, tmp); > + } > +} > + > /** > * update_cpumask - update the cpus_allowed mask of a cpuset and all tasks in it > * @cs: the cpuset to consider > @@ -2483,29 +2518,7 @@ static int update_cpumask(struct cpuset *cs, struct cpuset *trialcs, > */ > force = !cpumask_equal(cs->effective_xcpus, trialcs->effective_xcpus); > > - invalidate_cs_partition(trialcs); > - if (trialcs->prs_err) > - cs->prs_err = trialcs->prs_err; > - > - if (is_partition_valid(cs) || > - (is_partition_invalid(cs) && !trialcs->prs_err)) { > - struct cpumask *xcpus = trialcs->effective_xcpus; > - > - if (cpumask_empty(xcpus) && is_partition_invalid(cs)) > - xcpus = trialcs->cpus_allowed; This if statement was added in commit 46c521bac592 ("cgroup/cpuset: Enable invalid to valid local partition transition") that is missing in your new partition_cpus_change() function. Have you run the test_cpuset_prs.sh selftest with a patched kernel to make sure that there is no test failure? Cheers, Longman
On 2025/8/30 4:32, Waiman Long wrote: > On 8/28/25 8:56 AM, Chen Ridong wrote: >> From: Chen Ridong <chenridong@huawei.com> >> >> Refactor the partition_cpus_change function to handle both regular CPU >> set updates and exclusive CPU modifications, either of which may trigger >> partition state changes. This generalized function will also be utilized >> for exclusive CPU updates in subsequent patches. >> >> Signed-off-by: Chen Ridong <chenridong@huawei.com> >> --- >> kernel/cgroup/cpuset.c | 59 ++++++++++++++++++++++++++---------------- >> 1 file changed, 36 insertions(+), 23 deletions(-) >> >> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c >> index 75ad18ab40ae..e3eb87a33b12 100644 >> --- a/kernel/cgroup/cpuset.c >> +++ b/kernel/cgroup/cpuset.c >> @@ -2447,6 +2447,41 @@ static int acpus_validate_change(struct cpuset *cs, struct cpuset *trialcs, >> return retval; >> } >> +/** >> + * partition_cpus_change - Handle partition state changes due to CPU mask updates >> + * @cs: The target cpuset being modified >> + * @trialcs: The trial cpuset containing proposed configuration changes >> + * @tmp: Temporary masks for intermediate calculations >> + * >> + * This function handles partition state transitions triggered by CPU mask changes. >> + * CPU modifications may cause a partition to be disabled or require state updates. >> + */ >> +static void partition_cpus_change(struct cpuset *cs, struct cpuset *trialcs, >> + struct tmpmasks *tmp) >> +{ >> + if (cs_is_member(cs)) >> + return; >> + >> + invalidate_cs_partition(trialcs); >> + if (trialcs->prs_err) >> + cs->prs_err = trialcs->prs_err; >> + >> + if (is_remote_partition(cs)) { >> + if (trialcs->prs_err) >> + remote_partition_disable(cs, tmp); >> + else >> + remote_cpus_update(cs, trialcs->exclusive_cpus, >> + trialcs->effective_xcpus, tmp); >> + } else { >> + if (trialcs->prs_err) >> + update_parent_effective_cpumask(cs, partcmd_invalidate, >> + NULL, tmp); >> + else >> + update_parent_effective_cpumask(cs, partcmd_update, >> + trialcs->effective_xcpus, tmp); >> + } >> +} >> + >> /** >> * update_cpumask - update the cpus_allowed mask of a cpuset and all tasks in it >> * @cs: the cpuset to consider >> @@ -2483,29 +2518,7 @@ static int update_cpumask(struct cpuset *cs, struct cpuset *trialcs, >> */ >> force = !cpumask_equal(cs->effective_xcpus, trialcs->effective_xcpus); >> - invalidate_cs_partition(trialcs); >> - if (trialcs->prs_err) >> - cs->prs_err = trialcs->prs_err; >> - >> - if (is_partition_valid(cs) || >> - (is_partition_invalid(cs) && !trialcs->prs_err)) { >> - struct cpumask *xcpus = trialcs->effective_xcpus; >> - >> - if (cpumask_empty(xcpus) && is_partition_invalid(cs)) >> - xcpus = trialcs->cpus_allowed; > > This if statement was added in commit 46c521bac592 ("cgroup/cpuset: Enable invalid to valid local > partition transition") that is missing in your new partition_cpus_change() function. Have you run > the test_cpuset_prs.sh selftest with a patched kernel to make sure that there is no test failure? > > Cheers, > Longman Thank you Longman, I did run the self-test for every patch, and I appreciate the test script test_cpuset_prs.sh you provided. The trialcs->effective_xcpus will be updated using compute_trialcs_excpus, which was introduced in Patch 4. The corresponding logic was then added in Patch 5: - cpumask_and(excpus, user_xcpus(trialcs), parent->effective_xcpus); + /* trialcs is member, cpuset.cpus has no impact to excpus */ + if (cs_is_member(cs)) + cpumask_and(excpus, trialcs->exclusive_cpus, + parent->effective_xcpus); + else + cpumask_and(excpus, user_xcpus(trialcs), parent->effective_xcpus); + Therefore, as long as excpus is computed correctly, I believe this implementation can handle the scenario appropriately. -- Best regards, Ridong
On 8/29/25 10:01 PM, Chen Ridong wrote: > > On 2025/8/30 4:32, Waiman Long wrote: >> On 8/28/25 8:56 AM, Chen Ridong wrote: >>> From: Chen Ridong <chenridong@huawei.com> >>> >>> Refactor the partition_cpus_change function to handle both regular CPU >>> set updates and exclusive CPU modifications, either of which may trigger >>> partition state changes. This generalized function will also be utilized >>> for exclusive CPU updates in subsequent patches. >>> >>> Signed-off-by: Chen Ridong <chenridong@huawei.com> >>> --- >>> kernel/cgroup/cpuset.c | 59 ++++++++++++++++++++++++++---------------- >>> 1 file changed, 36 insertions(+), 23 deletions(-) >>> >>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c >>> index 75ad18ab40ae..e3eb87a33b12 100644 >>> --- a/kernel/cgroup/cpuset.c >>> +++ b/kernel/cgroup/cpuset.c >>> @@ -2447,6 +2447,41 @@ static int acpus_validate_change(struct cpuset *cs, struct cpuset *trialcs, >>> return retval; >>> } >>> +/** >>> + * partition_cpus_change - Handle partition state changes due to CPU mask updates >>> + * @cs: The target cpuset being modified >>> + * @trialcs: The trial cpuset containing proposed configuration changes >>> + * @tmp: Temporary masks for intermediate calculations >>> + * >>> + * This function handles partition state transitions triggered by CPU mask changes. >>> + * CPU modifications may cause a partition to be disabled or require state updates. >>> + */ >>> +static void partition_cpus_change(struct cpuset *cs, struct cpuset *trialcs, >>> + struct tmpmasks *tmp) >>> +{ >>> + if (cs_is_member(cs)) >>> + return; >>> + >>> + invalidate_cs_partition(trialcs); >>> + if (trialcs->prs_err) >>> + cs->prs_err = trialcs->prs_err; >>> + >>> + if (is_remote_partition(cs)) { >>> + if (trialcs->prs_err) >>> + remote_partition_disable(cs, tmp); >>> + else >>> + remote_cpus_update(cs, trialcs->exclusive_cpus, >>> + trialcs->effective_xcpus, tmp); >>> + } else { >>> + if (trialcs->prs_err) >>> + update_parent_effective_cpumask(cs, partcmd_invalidate, >>> + NULL, tmp); >>> + else >>> + update_parent_effective_cpumask(cs, partcmd_update, >>> + trialcs->effective_xcpus, tmp); >>> + } >>> +} >>> + >>> /** >>> * update_cpumask - update the cpus_allowed mask of a cpuset and all tasks in it >>> * @cs: the cpuset to consider >>> @@ -2483,29 +2518,7 @@ static int update_cpumask(struct cpuset *cs, struct cpuset *trialcs, >>> */ >>> force = !cpumask_equal(cs->effective_xcpus, trialcs->effective_xcpus); >>> - invalidate_cs_partition(trialcs); >>> - if (trialcs->prs_err) >>> - cs->prs_err = trialcs->prs_err; >>> - >>> - if (is_partition_valid(cs) || >>> - (is_partition_invalid(cs) && !trialcs->prs_err)) { >>> - struct cpumask *xcpus = trialcs->effective_xcpus; >>> - >>> - if (cpumask_empty(xcpus) && is_partition_invalid(cs)) >>> - xcpus = trialcs->cpus_allowed; >> This if statement was added in commit 46c521bac592 ("cgroup/cpuset: Enable invalid to valid local >> partition transition") that is missing in your new partition_cpus_change() function. Have you run >> the test_cpuset_prs.sh selftest with a patched kernel to make sure that there is no test failure? >> >> Cheers, >> Longman > Thank you Longman, > > I did run the self-test for every patch, and I appreciate the test script test_cpuset_prs.sh you > provided. > > The trialcs->effective_xcpus will be updated using compute_trialcs_excpus, which was introduced in > Patch 4. The corresponding logic was then added in Patch 5: > > - cpumask_and(excpus, user_xcpus(trialcs), parent->effective_xcpus); > + /* trialcs is member, cpuset.cpus has no impact to excpus */ > + if (cs_is_member(cs)) > + cpumask_and(excpus, trialcs->exclusive_cpus, > + parent->effective_xcpus); > + else > + cpumask_and(excpus, user_xcpus(trialcs), parent->effective_xcpus); > + > > Therefore, as long as excpus is computed correctly, I believe this implementation can handle the > scenario appropriately. It will be helpful to put down a note in the commit log that the missing logic will be re-introduced in a subsequent patch. Thanks, Longman
On 2025/9/2 21:30, Waiman Long wrote: > On 8/29/25 10:01 PM, Chen Ridong wrote: >> >> On 2025/8/30 4:32, Waiman Long wrote: >>> On 8/28/25 8:56 AM, Chen Ridong wrote: >>>> From: Chen Ridong <chenridong@huawei.com> >>>> >>>> Refactor the partition_cpus_change function to handle both regular CPU >>>> set updates and exclusive CPU modifications, either of which may trigger >>>> partition state changes. This generalized function will also be utilized >>>> for exclusive CPU updates in subsequent patches. >>>> >>>> Signed-off-by: Chen Ridong <chenridong@huawei.com> >>>> --- >>>> kernel/cgroup/cpuset.c | 59 ++++++++++++++++++++++++++---------------- >>>> 1 file changed, 36 insertions(+), 23 deletions(-) >>>> >>>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c >>>> index 75ad18ab40ae..e3eb87a33b12 100644 >>>> --- a/kernel/cgroup/cpuset.c >>>> +++ b/kernel/cgroup/cpuset.c >>>> @@ -2447,6 +2447,41 @@ static int acpus_validate_change(struct cpuset *cs, struct cpuset *trialcs, >>>> return retval; >>>> } >>>> +/** >>>> + * partition_cpus_change - Handle partition state changes due to CPU mask updates >>>> + * @cs: The target cpuset being modified >>>> + * @trialcs: The trial cpuset containing proposed configuration changes >>>> + * @tmp: Temporary masks for intermediate calculations >>>> + * >>>> + * This function handles partition state transitions triggered by CPU mask changes. >>>> + * CPU modifications may cause a partition to be disabled or require state updates. >>>> + */ >>>> +static void partition_cpus_change(struct cpuset *cs, struct cpuset *trialcs, >>>> + struct tmpmasks *tmp) >>>> +{ >>>> + if (cs_is_member(cs)) >>>> + return; >>>> + >>>> + invalidate_cs_partition(trialcs); >>>> + if (trialcs->prs_err) >>>> + cs->prs_err = trialcs->prs_err; >>>> + >>>> + if (is_remote_partition(cs)) { >>>> + if (trialcs->prs_err) >>>> + remote_partition_disable(cs, tmp); >>>> + else >>>> + remote_cpus_update(cs, trialcs->exclusive_cpus, >>>> + trialcs->effective_xcpus, tmp); >>>> + } else { >>>> + if (trialcs->prs_err) >>>> + update_parent_effective_cpumask(cs, partcmd_invalidate, >>>> + NULL, tmp); >>>> + else >>>> + update_parent_effective_cpumask(cs, partcmd_update, >>>> + trialcs->effective_xcpus, tmp); >>>> + } >>>> +} >>>> + >>>> /** >>>> * update_cpumask - update the cpus_allowed mask of a cpuset and all tasks in it >>>> * @cs: the cpuset to consider >>>> @@ -2483,29 +2518,7 @@ static int update_cpumask(struct cpuset *cs, struct cpuset *trialcs, >>>> */ >>>> force = !cpumask_equal(cs->effective_xcpus, trialcs->effective_xcpus); >>>> - invalidate_cs_partition(trialcs); >>>> - if (trialcs->prs_err) >>>> - cs->prs_err = trialcs->prs_err; >>>> - >>>> - if (is_partition_valid(cs) || >>>> - (is_partition_invalid(cs) && !trialcs->prs_err)) { >>>> - struct cpumask *xcpus = trialcs->effective_xcpus; >>>> - >>>> - if (cpumask_empty(xcpus) && is_partition_invalid(cs)) >>>> - xcpus = trialcs->cpus_allowed; >>> This if statement was added in commit 46c521bac592 ("cgroup/cpuset: Enable invalid to valid local >>> partition transition") that is missing in your new partition_cpus_change() function. Have you run >>> the test_cpuset_prs.sh selftest with a patched kernel to make sure that there is no test failure? >>> >>> Cheers, >>> Longman >> Thank you Longman, >> >> I did run the self-test for every patch, and I appreciate the test script test_cpuset_prs.sh you >> provided. >> >> The trialcs->effective_xcpus will be updated using compute_trialcs_excpus, which was introduced in >> Patch 4. The corresponding logic was then added in Patch 5: >> >> - cpumask_and(excpus, user_xcpus(trialcs), parent->effective_xcpus); >> + /* trialcs is member, cpuset.cpus has no impact to excpus */ >> + if (cs_is_member(cs)) >> + cpumask_and(excpus, trialcs->exclusive_cpus, >> + parent->effective_xcpus); >> + else >> + cpumask_and(excpus, user_xcpus(trialcs), parent->effective_xcpus); >> + >> >> Therefore, as long as excpus is computed correctly, I believe this implementation can handle the >> scenario appropriately. > > It will be helpful to put down a note in the commit log that the missing logic will be re-introduced > in a subsequent patch. > > Thanks, > Longman Thank you Longman, I will update the commit message. -- Best regards, Ridong
© 2016 - 2025 Red Hat, Inc.