Message-ID: <CANMuvJkDjuPpcqMBM+zzNL3wA-1zVrshrMuy22kQKmLDxbsB7Q@mail.gmail.com>
Date: Mon, 15 Jul 2024 08:56:55 -0700
From: Petr Malat <oss@...at.biz>
To: Waiman Long <longman@...hat.com>
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

Hi,
I finally got some time to test this and it works exactly as we needed it to.
Thanks a lot,
  Petr

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