v4:
- Add missing rcu_read_lock/unlock to cpuset_cancel_fork() in patch 3.
- Add patch 5 to reduce performance impact for the
non-CLONE_INTO_CGROUP case.
v3:
- Update patches 2 & 3 to put task_cs() call under rcu_read_lock().
v2:
- Drop v1 patch 3
- Add a new patch to fix an issue in cpuset_cancel_attach() and
another patch to add cpuset_can_fork() and cpuset_cacnel_fork()
methods.
The first patch in this series fixes a problem in
cpuset_cancel_attach(). Patches 2 and 3 fixes the CLONE_INTO_CGROUP
problem in cpuset. Patch 4 is a minor fix. The last patch is a
performance optimization patch for the non-CLONE_INTO_CGROUP case.
Waiman Long (5):
cgroup/cpuset: Wake up cpuset_attach_wq tasks in
cpuset_cancel_attach()
cgroup/cpuset: Make cpuset_fork() handle CLONE_INTO_CGROUP properly
cgroup/cpuset: Add cpuset_can_fork() and cpuset_cancel_fork() methods
cgroup/cpuset: Make cpuset_attach_task() skip subpartitions CPUs for
top_cpuset
cgroup/cpuset: Optimize out unneeded
cpuset_can_fork/cpuset_cancel_fork calls
include/linux/cgroup-defs.h | 6 ++
kernel/cgroup/cgroup.c | 23 +++--
kernel/cgroup/cpuset.c | 167 +++++++++++++++++++++++++++++-------
3 files changed, 159 insertions(+), 37 deletions(-)
--
2.31.1
After a successful cpuset_can_attach() call which increments the
attach_in_progress flag, either cpuset_cancel_attach() or cpuset_attach()
will be called later. In cpuset_attach(), tasks in cpuset_attach_wq,
if present, will be woken up at the end. That is not the case in
cpuset_cancel_attach(). So missed wakeup is possible if the attach
operation is somehow cancelled. Fix that by doing the wakeup in
cpuset_cancel_attach() as well.
Fixes: e44193d39e8d ("cpuset: let hotplug propagation work wait for task attaching")
Signed-off-by: Waiman Long <[email protected]>
Reviewed-by: Michal Koutný <[email protected]>
---
kernel/cgroup/cpuset.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index bc4dcfd7bee5..066689a7dcc3 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -2507,11 +2507,15 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
static void cpuset_cancel_attach(struct cgroup_taskset *tset)
{
struct cgroup_subsys_state *css;
+ struct cpuset *cs;
cgroup_taskset_first(tset, &css);
+ cs = css_cs(css);
percpu_down_write(&cpuset_rwsem);
- css_cs(css)->attach_in_progress--;
+ cs->attach_in_progress--;
+ if (!cs->attach_in_progress)
+ wake_up(&cpuset_attach_wq);
percpu_up_write(&cpuset_rwsem);
}
--
2.31.1
In the case of CLONE_INTO_CGROUP, not all cpusets are ready to accept
new tasks. It is too late to check that in cpuset_fork(). So we need
to add the cpuset_can_fork() and cpuset_cancel_fork() methods to
pre-check it before we can allow attachment to a different cpuset.
We also need to set the attach_in_progress flag to alert other code
that a new task is going to be added to the cpuset.
Fixes: ef2c41cf38a7 ("clone3: allow spawning processes into cgroups")
Suggested-by: Michal Koutný <[email protected]>
Signed-off-by: Waiman Long <[email protected]>
---
kernel/cgroup/cpuset.c | 97 +++++++++++++++++++++++++++++++++++++-----
1 file changed, 86 insertions(+), 11 deletions(-)
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index e954d5abb784..132cdae4f03c 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -2458,6 +2458,20 @@ static int fmeter_getrate(struct fmeter *fmp)
static struct cpuset *cpuset_attach_old_cs;
+/*
+ * Check to see if a cpuset can accept a new task
+ * For v1, cpus_allowed and mems_allowed can't be empty.
+ * For v2, effective_cpus can't be empty.
+ * Note that in v1, effective_cpus = cpus_allowed.
+ */
+static int cpuset_can_attach_check(struct cpuset *cs)
+{
+ if (cpumask_empty(cs->effective_cpus) ||
+ (!is_in_v2_mode() && nodes_empty(cs->mems_allowed)))
+ return -ENOSPC;
+ return 0;
+}
+
/* Called by cgroups to determine if a cpuset is usable; cpuset_rwsem held */
static int cpuset_can_attach(struct cgroup_taskset *tset)
{
@@ -2472,16 +2486,9 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
percpu_down_write(&cpuset_rwsem);
- /* allow moving tasks into an empty cpuset if on default hierarchy */
- ret = -ENOSPC;
- if (!is_in_v2_mode() &&
- (cpumask_empty(cs->cpus_allowed) || nodes_empty(cs->mems_allowed)))
- goto out_unlock;
-
- /*
- * Task cannot be moved to a cpuset with empty effective cpus.
- */
- if (cpumask_empty(cs->effective_cpus))
+ /* Check to see if task is allowed in the cpuset */
+ ret = cpuset_can_attach_check(cs);
+ if (ret)
goto out_unlock;
cgroup_taskset_for_each(task, css, tset) {
@@ -2498,7 +2505,6 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
* changes which zero cpus/mems_allowed.
*/
cs->attach_in_progress++;
- ret = 0;
out_unlock:
percpu_up_write(&cpuset_rwsem);
return ret;
@@ -3269,6 +3275,68 @@ static void cpuset_bind(struct cgroup_subsys_state *root_css)
percpu_up_write(&cpuset_rwsem);
}
+/*
+ * In case the child is cloned into a cpuset different from its parent,
+ * additional checks are done to see if the move is allowed.
+ */
+static int cpuset_can_fork(struct task_struct *task, struct css_set *cset)
+{
+ struct cpuset *cs = css_cs(cset->subsys[cpuset_cgrp_id]);
+ bool same_cs;
+ int ret;
+
+ rcu_read_lock();
+ same_cs = (cs == task_cs(current));
+ rcu_read_unlock();
+
+ if (same_cs)
+ return 0;
+
+ lockdep_assert_held(&cgroup_mutex);
+ percpu_down_write(&cpuset_rwsem);
+
+ /* Check to see if task is allowed in the cpuset */
+ ret = cpuset_can_attach_check(cs);
+ if (ret)
+ goto out_unlock;
+
+ ret = task_can_attach(task, cs->effective_cpus);
+ if (ret)
+ goto out_unlock;
+
+ ret = security_task_setscheduler(task);
+ if (ret)
+ goto out_unlock;
+
+ /*
+ * Mark attach is in progress. This makes validate_change() fail
+ * changes which zero cpus/mems_allowed.
+ */
+ cs->attach_in_progress++;
+out_unlock:
+ percpu_up_write(&cpuset_rwsem);
+ return ret;
+}
+
+static void cpuset_cancel_fork(struct task_struct *task, struct css_set *cset)
+{
+ struct cpuset *cs = css_cs(cset->subsys[cpuset_cgrp_id]);
+ bool same_cs;
+
+ rcu_read_lock();
+ same_cs = (cs == task_cs(current));
+ rcu_read_unlock();
+
+ if (same_cs)
+ return;
+
+ percpu_down_write(&cpuset_rwsem);
+ cs->attach_in_progress--;
+ if (!cs->attach_in_progress)
+ wake_up(&cpuset_attach_wq);
+ percpu_up_write(&cpuset_rwsem);
+}
+
/*
* Make sure the new task conform to the current state of its parent,
* which could have been changed by cpuset just after it inherits the
@@ -3297,6 +3365,11 @@ static void cpuset_fork(struct task_struct *task)
percpu_down_write(&cpuset_rwsem);
guarantee_online_mems(cs, &cpuset_attach_nodemask_to);
cpuset_attach_task(cs, task);
+
+ cs->attach_in_progress--;
+ if (!cs->attach_in_progress)
+ wake_up(&cpuset_attach_wq);
+
percpu_up_write(&cpuset_rwsem);
}
@@ -3310,6 +3383,8 @@ struct cgroup_subsys cpuset_cgrp_subsys = {
.attach = cpuset_attach,
.post_attach = cpuset_post_attach,
.bind = cpuset_bind,
+ .can_fork = cpuset_can_fork,
+ .cancel_fork = cpuset_cancel_fork,
.fork = cpuset_fork,
.legacy_cftypes = legacy_files,
.dfl_cftypes = dfl_files,
--
2.31.1
It is found that attaching a task to the top_cpuset does not currently
ignore CPUs allocated to subpartitions in cpuset_attach_task(). So the
code is changed to fix that.
Signed-off-by: Waiman Long <[email protected]>
Reviewed-by: Michal Koutný <[email protected]>
---
kernel/cgroup/cpuset.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 132cdae4f03c..e4ca2dd2b764 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -2540,7 +2540,8 @@ static void cpuset_attach_task(struct cpuset *cs, struct task_struct *task)
if (cs != &top_cpuset)
guarantee_online_cpus(task, cpus_attach);
else
- cpumask_copy(cpus_attach, task_cpu_possible_mask(task));
+ cpumask_andnot(cpus_attach, task_cpu_possible_mask(task),
+ cs->subparts_cpus);
/*
* can_attach beforehand should guarantee that this doesn't
* fail. TODO: have a better way to handle failure here
--
2.31.1
On Tue, Apr 11, 2023 at 09:35:56AM -0400, Waiman Long wrote:
> v4:
> - Add missing rcu_read_lock/unlock to cpuset_cancel_fork() in patch 3.
> - Add patch 5 to reduce performance impact for the
> non-CLONE_INTO_CGROUP case.
>
> v3:
> - Update patches 2 & 3 to put task_cs() call under rcu_read_lock().
>
> v2:
> - Drop v1 patch 3
> - Add a new patch to fix an issue in cpuset_cancel_attach() and
> another patch to add cpuset_can_fork() and cpuset_cacnel_fork()
> methods.
>
> The first patch in this series fixes a problem in
> cpuset_cancel_attach(). Patches 2 and 3 fixes the CLONE_INTO_CGROUP
> problem in cpuset. Patch 4 is a minor fix. The last patch is a
> performance optimization patch for the non-CLONE_INTO_CGROUP case.
Applied 1-4 to cgroup/for-6.3-fixes w/ stable cc'd. Given that the fixes are
a bit involved, the breakages have been there for quite a while and the
cpuset code has changed quite a bit, backporting might not be trivial tho.
Let's see how that goes.
Thanks.
--
tejun
On 4/12/23 14:26, Tejun Heo wrote:
> On Tue, Apr 11, 2023 at 09:35:56AM -0400, Waiman Long wrote:
>> v4:
>> - Add missing rcu_read_lock/unlock to cpuset_cancel_fork() in patch 3.
>> - Add patch 5 to reduce performance impact for the
>> non-CLONE_INTO_CGROUP case.
>>
>> v3:
>> - Update patches 2 & 3 to put task_cs() call under rcu_read_lock().
>>
>> v2:
>> - Drop v1 patch 3
>> - Add a new patch to fix an issue in cpuset_cancel_attach() and
>> another patch to add cpuset_can_fork() and cpuset_cacnel_fork()
>> methods.
>>
>> The first patch in this series fixes a problem in
>> cpuset_cancel_attach(). Patches 2 and 3 fixes the CLONE_INTO_CGROUP
>> problem in cpuset. Patch 4 is a minor fix. The last patch is a
>> performance optimization patch for the non-CLONE_INTO_CGROUP case.
> Applied 1-4 to cgroup/for-6.3-fixes w/ stable cc'd. Given that the fixes are
> a bit involved, the breakages have been there for quite a while and the
> cpuset code has changed quite a bit, backporting might not be trivial tho.
> Let's see how that goes.
I know stable backport won't be straight forward. I am planning to help
in the backporting effort. Thanks for taking these into your cgroup tree.
Cheers,
Longman