Message-ID: <e41a9286-8103-4897-83fc-f9185f4488e3@redhat.com>
Date: Mon, 15 Jul 2024 15:17:51 -0400
From: Waiman Long <longman@...hat.com>
To: Petr Malat <oss@...at.biz>
Cc: Tejun Heo <tj@...nel.org>, Zefan Li <lizefan.x@...edance.com>,
 Johannes Weiner <hannes@...xchg.org>, Jonathan Corbet <corbet@....net>,
 Shuah Khan <shuah@...nel.org>, cgroups@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
 linux-kselftest@...r.kernel.org, Xavier <ghostxavier@...a.com>,
 Peter Hunt <pehunt@...hat.com>
Subject: Re: [PATCH-cgroup v2 4/5] cgroup/cpuset: Make cpuset.cpus.exclusive
 independent of cpuset.cpus

On 7/15/24 11:56, Petr Malat wrote:
> Hi,
> I finally got some time to test this and it works exactly as we needed it to.
> Thanks a lot,
>    Petr

Thanks for the verification.

Cheers,
Longman

> On Mon, Jun 17, 2024 at 10:39:44AM -0400, Waiman Long wrote:
>> The "cpuset.cpus.exclusive.effective" value is currently limited to a
>> subset of its "cpuset.cpus". This makes the exclusive CPUs distribution
>> hierarchy subsumed within the larger "cpuset.cpus" hierarchy. We have to
>> decide on what CPUs are used locally and what CPUs can be passed down as
>> exclusive CPUs down the hierarchy and combine them into "cpuset.cpus".
>>
>> The advantage of the current scheme is to have only one hierarchy to
>> worry about. However, it make it harder to use as all the "cpuset.cpus"
>> values have to be properly set along the way down to the designated remote
>> partition root. It also makes it more cumbersome to find out what CPUs
>> can be used locally.
>>
>> Make creation of remote partition simpler by breaking the
>> dependency of "cpuset.cpus.exclusive" on "cpuset.cpus" and make
>> them independent entities. Now we have two separate hierarchies -
>> one for setting "cpuset.cpus.effective" and the other one for setting
>> "cpuset.cpus.exclusive.effective". We may not need to set "cpuset.cpus"
>> when we activate a partition root anymore.
>>
>> Also update Documentation/admin-guide/cgroup-v2.rst and cpuset.c comment
>> to document this change.
>>
>> Suggested-by: Petr Malat <oss@...at.biz>
>> Signed-off-by: Waiman Long <longman@...hat.com>
>> ---
>>   Documentation/admin-guide/cgroup-v2.rst |  4 +-
>>   kernel/cgroup/cpuset.c                  | 67 +++++++++++++++++--------
>>   2 files changed, 49 insertions(+), 22 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
>> index 722e4762c4e0..2e4e74bea6ef 100644
>> --- a/Documentation/admin-guide/cgroup-v2.rst
>> +++ b/Documentation/admin-guide/cgroup-v2.rst
>> @@ -2380,8 +2380,8 @@ Cpuset Interface Files
>>   	cpuset-enabled cgroups.
>>
>>   	This file shows the effective set of exclusive CPUs that
>> -	can be used to create a partition root.  The content of this
>> -	file will always be a subset of "cpuset.cpus" and its parent's
>> +	can be used to create a partition root.  The content
>> +	of this file will always be a subset of its parent's
>>   	"cpuset.cpus.exclusive.effective" if its parent is not the root
>>   	cgroup.  It will also be a subset of "cpuset.cpus.exclusive"
>>   	if it is set.  If "cpuset.cpus.exclusive" is not set, it is
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index 144bfc319809..fe76045aa528 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -87,7 +87,7 @@ static const char * const perr_strings[] = {
>>   	[PERR_NOTEXCL]   = "Cpu list in cpuset.cpus not exclusive",
>>   	[PERR_NOCPUS]    = "Parent unable to distribute cpu downstream",
>>   	[PERR_HOTPLUG]   = "No cpu available due to hotplug",
>> -	[PERR_CPUSEMPTY] = "cpuset.cpus is empty",
>> +	[PERR_CPUSEMPTY] = "cpuset.cpus and cpuset.cpus.exclusive are empty",
>>   	[PERR_HKEEPING]  = "partition config conflicts with housekeeping setup",
>>   };
>>
>> @@ -127,19 +127,28 @@ struct cpuset {
>>   	/*
>>   	 * Exclusive CPUs dedicated to current cgroup (default hierarchy only)
>>   	 *
>> -	 * This exclusive CPUs must be a subset of cpus_allowed. A parent
>> -	 * cgroup can only grant exclusive CPUs to one of its children.
>> +	 * The effective_cpus of a valid partition root comes solely from its
>> +	 * effective_xcpus and some of the effective_xcpus may be distributed
>> +	 * to sub-partitions below & hence excluded from its effective_cpus.
>> +	 * For a valid partition root, its effective_cpus have no relationship
>> +	 * with cpus_allowed unless its exclusive_cpus isn't set.
>>   	 *
>> -	 * When the cgroup becomes a valid partition root, effective_xcpus
>> -	 * defaults to cpus_allowed if not set. The effective_cpus of a valid
>> -	 * partition root comes solely from its effective_xcpus and some of the
>> -	 * effective_xcpus may be distributed to sub-partitions below & hence
>> -	 * excluded from its effective_cpus.
>> +	 * This value will only be set if either exclusive_cpus is set or
>> +	 * when this cpuset becomes a local partition root.
>>   	 */
>>   	cpumask_var_t effective_xcpus;
>>
>>   	/*
>>   	 * Exclusive CPUs as requested by the user (default hierarchy only)
>> +	 *
>> +	 * Its value is independent of cpus_allowed and designates the set of
>> +	 * CPUs that can be granted to the current cpuset or its children when
>> +	 * it becomes a valid partition root. The effective set of exclusive
>> +	 * CPUs granted (effective_xcpus) depends on whether those exclusive
>> +	 * CPUs are passed down by its ancestors and not yet taken up by
>> +	 * another sibling partition root along the way.
>> +	 *
>> +	 * If its value isn't set, it defaults to cpus_allowed.
>>   	 */
>>   	cpumask_var_t exclusive_cpus;
>>
>> @@ -230,6 +239,17 @@ static struct list_head remote_children;
>>    *   2 - partition root without load balancing (isolated)
>>    *  -1 - invalid partition root
>>    *  -2 - invalid isolated partition root
>> + *
>> + *  There are 2 types of partitions - local or remote. Local partitions are
>> + *  those whose parents are partition root themselves. Setting of
>> + *  cpuset.cpus.exclusive are optional in setting up local partitions.
>> + *  Remote partitions are those whose parents are not partition roots. Passing
>> + *  down exclusive CPUs by setting cpuset.cpus.exclusive along its ancestor
>> + *  nodes are mandatory in creating a remote partition.
>> + *
>> + *  For simplicity, a local partition can be created under a local or remote
>> + *  partition but a remote partition cannot have any partition root in its
>> + *  ancestor chain except the cgroup root.
>>    */
>>   #define PRS_MEMBER		0
>>   #define PRS_ROOT		1
>> @@ -709,6 +729,19 @@ static inline void free_cpuset(struct cpuset *cs)
>>   	kfree(cs);
>>   }
>>
>> +/* Return user specified exclusive CPUs */
>> +static inline struct cpumask *user_xcpus(struct cpuset *cs)
>> +{
>> +	return cpumask_empty(cs->exclusive_cpus) ? cs->cpus_allowed
>> +						 : cs->exclusive_cpus;
>> +}
>> +
>> +static inline bool xcpus_empty(struct cpuset *cs)
>> +{
>> +	return cpumask_empty(cs->cpus_allowed) &&
>> +	       cpumask_empty(cs->exclusive_cpus);
>> +}
>> +
>>   static inline struct cpumask *fetch_xcpus(struct cpuset *cs)
>>   {
>>   	return !cpumask_empty(cs->exclusive_cpus) ? cs->exclusive_cpus :
>> @@ -1593,7 +1626,7 @@ EXPORT_SYMBOL_GPL(cpuset_cpu_is_isolated);
>>    * Return: true if xcpus is not empty, false otherwise.
>>    *
>>    * Starting with exclusive_cpus (cpus_allowed if exclusive_cpus is not set),
>> - * it must be a subset of cpus_allowed and parent's effective_xcpus.
>> + * it must be a subset of parent's effective_xcpus.
>>    */
>>   static bool compute_effective_exclusive_cpumask(struct cpuset *cs,
>>   						struct cpumask *xcpus)
>> @@ -1603,12 +1636,7 @@ static bool compute_effective_exclusive_cpumask(struct cpuset *cs,
>>   	if (!xcpus)
>>   		xcpus = cs->effective_xcpus;
>>
>> -	if (!cpumask_empty(cs->exclusive_cpus))
>> -		cpumask_and(xcpus, cs->exclusive_cpus, cs->cpus_allowed);
>> -	else
>> -		cpumask_copy(xcpus, cs->cpus_allowed);
>> -
>> -	return cpumask_and(xcpus, xcpus, parent->effective_xcpus);
>> +	return cpumask_and(xcpus, user_xcpus(cs), parent->effective_xcpus);
>>   }
>>
>>   static inline bool is_remote_partition(struct cpuset *cs)
>> @@ -1887,8 +1915,7 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>>   	 */
>>   	adding = deleting = false;
>>   	old_prs = new_prs = cs->partition_root_state;
>> -	xcpus = !cpumask_empty(cs->exclusive_cpus)
>> -		? cs->effective_xcpus : cs->cpus_allowed;
>> +	xcpus = user_xcpus(cs);
>>
>>   	if (cmd == partcmd_invalidate) {
>>   		if (is_prs_invalid(old_prs))
>> @@ -1916,7 +1943,7 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>>   		return is_partition_invalid(parent)
>>   		       ? PERR_INVPARENT : PERR_NOTPART;
>>   	}
>> -	if (!newmask && cpumask_empty(cs->cpus_allowed))
>> +	if (!newmask && xcpus_empty(cs))
>>   		return PERR_CPUSEMPTY;
>>
>>   	nocpu = tasks_nocpu_error(parent, cs, xcpus);
>> @@ -3130,9 +3157,9 @@ static int update_prstate(struct cpuset *cs, int new_prs)
>>   				       ? partcmd_enable : partcmd_enablei;
>>
>>   		/*
>> -		 * cpus_allowed cannot be empty.
>> +		 * cpus_allowed and exclusive_cpus cannot be both empty.
>>   		 */
>> -		if (cpumask_empty(cs->cpus_allowed)) {
>> +		if (xcpus_empty(cs)) {
>>   			err = PERR_CPUSEMPTY;
>>   			goto out;
>>   		}
>> --
>> 2.39.3
>>

Powered by blists - more mailing lists