Linux cgroups development
 help / color / mirror / Atom feed
From: Guopeng Zhang <guopeng.zhang@linux.dev>
To: "Waiman Long" <longman@redhat.com>,
	"Chen Ridong" <chenridong@huaweicloud.com>,
	"Tejun Heo" <tj@kernel.org>,
	"Johannes Weiner" <hannes@cmpxchg.org>,
	"Michal Koutný" <mkoutny@suse.com>,
	"Ingo Molnar" <mingo@redhat.com>,
	"Peter Zijlstra" <peterz@infradead.org>
Cc: cgroups@vger.kernel.org, linux-kernel@vger.kernel.org,
	Aaron Tomlin <atomlin@atomlin.com>
Subject: Re: [PATCH-next v3 5/5] cgroup/cpuset: Support multiple source/destination cpusets for cpuset_*attach()
Date: Fri, 29 May 2026 10:26:36 +0800	[thread overview]
Message-ID: <e6ede505-e9a3-49a0-bfa5-e624c0aa7d6a@linux.dev> (raw)
In-Reply-To: <20260527153800.1557449-6-longman@redhat.com>



在 2026/5/27 23:38, Waiman Long 写道:
> With cgroup v2, the cgroup_taskset structure passed into the cgroup
> can_attach() and attach() methods can contain task migration data with
> multiple destination or source cpusets when the cpuset controller is
> enabled or disabled respectively.
...
> -/* Called by cgroups to determine if a cpuset is usable; cpuset_mutex held */
> +/*
> + * Called by cgroups to determine if a cpuset is usable; cpuset_mutex held.
> + *
> + * With cgroup v2, enabling of cpuset controller in a cgroup subtree can
> + * cause @tset to contain task migration data from one parent cpuset to multiple
> + * child cpusets. Not much is needed to be done here other than tracking the
> + * number of DL tasks in each cpuset as the CPUs and memory nodes of the child
> + * cpusets are exactly the same as the parent.
> + *
> + * Conversely, disabling of cpuset controller can cause @tset to contain task
> + * migration data from multiple child cpusets to one parent cpuset. Here, the
> + * CPUs and memory nodes of the child cpusets may be different from the parent,
> + * but must be a subset of its parent.
> + *
> + * Another possible many-to-one migration is the moving of the whole
> + * multithreaded process with threads in different cpusets to another cpuset.
> + *
> + * For all other use cases, @tset task migration data should be from one source
> + * cpuset to one destination cpuset.
> + */
>  static int cpuset_can_attach(struct cgroup_taskset *tset)
>  {
>  	struct cgroup_subsys_state *css;
> @@ -3079,6 +3172,16 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
>  		goto out_unlock;
>  
>  	cgroup_taskset_for_each(task, css, tset) {
> +		struct cpuset *newcs = css_cs(css);
> +		struct cpuset *new_oldcs = task_cs(task);
> +
> +		if ((newcs != cs) || (new_oldcs != oldcs)) {
> +			cs = newcs;
> +			oldcs = new_oldcs;
> +			ret = cpuset_can_attach_check(cs, oldcs, &setsched_check);
> +			if (ret)
> +				goto out_unlock;
> +		}
Just a minor nit while running checkpatch --strict on this patch:

checkpatch reports unnecessary parentheses here:

if ((newcs != cs) || (new_oldcs != oldcs)) {

Perhaps this can be simplified to:

if (newcs != cs || new_oldcs != oldcs) {
>  		ret = task_can_attach(task);
>  		if (ret)
...
>  	/*
>  	 * In the default hierarchy, enabling cpuset in the child cgroups
> -	 * will trigger a number of cpuset_attach() calls with no change
> -	 * in effective cpus and mems. In that case, we can optimize out
> -	 * by skipping the task iteration and update.
> +	 * will trigger a cpuset_attach() call with no change in effective cpus
> +	 * and mems. In that case, we can optimize out by skipping the task
> +	 * iteration and update, but the destination cpuset list is iterated to
> +	 * set old_mems_sllowed.
>  	 */
I also noticed one small typo in the added comment:

s/old_mems_sllowed/old_mems_allowed/

Best,
Guopeng



      reply	other threads:[~2026-05-29  2:26 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-27 15:37 [PATCH-next v3 0/5] cgroup/cpuset: Support multiple source/destination cpusets for cpuset_*attach() Waiman Long
2026-05-27 15:37 ` [PATCH-next v3 1/5] cgroup/cpuset: Add a cpuset_reserve_dl_bw() helper Waiman Long
2026-05-27 15:37 ` [PATCH-next v3 2/5] cgroup/cpuset: Expand the scope of cpuset_can_attach_check() Waiman Long
2026-05-27 15:37 ` [PATCH-next v3 3/5] cgroup/cpuset: Made cpuset_attach_old_cs track task group leaders Waiman Long
2026-05-29  2:19   ` Guopeng Zhang
2026-05-29 16:54     ` Waiman Long
2026-05-27 15:37 ` [PATCH-next v3 4/5] cgroup/cpuset: Move mpol_rebind_mm/cpuset_migrate_mm() calls inside cpuset_attach_task() Waiman Long
2026-05-29  2:21   ` Guopeng Zhang
2026-05-27 15:38 ` [PATCH-next v3 5/5] cgroup/cpuset: Support multiple source/destination cpusets for cpuset_*attach() Waiman Long
2026-05-29  2:26   ` Guopeng Zhang [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e6ede505-e9a3-49a0-bfa5-e624c0aa7d6a@linux.dev \
    --to=guopeng.zhang@linux.dev \
    --cc=atomlin@atomlin.com \
    --cc=cgroups@vger.kernel.org \
    --cc=chenridong@huaweicloud.com \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=mingo@redhat.com \
    --cc=mkoutny@suse.com \
    --cc=peterz@infradead.org \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox