2015-11-25 22:01:54

by Marcelo Tosatti

Subject: [PATCH] cgroups: move cpuset specific checks from generic code to cpuset_can_attach

Move PF_NO_SETAFFINITY check to cpuset cgroups, where it belongs.
This makes it possible to attach PF_NO_SETAFFINITY to Intel CAT cgroups.

Reported-by: Luiz Capitulino <[email protected]>
Signed-off-by: Marcelo Tosatti <[email protected]>

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index f89d929..0603652 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -2466,16 +2466,6 @@ static ssize_t __cgroup_procs_write(struct kernfs_open_file *of, char *buf,
if (threadgroup)
tsk = tsk->group_leader;

- /*
- * Workqueue threads may acquire PF_NO_SETAFFINITY and become
- * trapped in a cpuset, or RT worker may be born in a cgroup
- * with no rt_runtime allocated. Just say no.
- */
- if (tsk == kthreadd_task || (tsk->flags & PF_NO_SETAFFINITY)) {
- ret = -EINVAL;
- goto out_unlock_rcu;
- }
-
get_task_struct(tsk);
rcu_read_unlock();

diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index f0acff0..9fff4d6 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -53,6 +53,7 @@
#include <linux/time.h>
#include <linux/backing-dev.h>
#include <linux/sort.h>
+#include <linux/kthread.h>

#include <asm/uaccess.h>
#include <linux/atomic.h>
@@ -1445,6 +1446,17 @@ static int cpuset_can_attach(struct cgroup_subsys_state *css,
goto out_unlock;

cgroup_taskset_for_each(task, tset) {
+ /*
+ * Workqueue threads may acquire PF_NO_SETAFFINITY and become
+ * trapped in a cpuset, or RT worker may be born in a cgroup
+ * with no rt_runtime allocated. Just say no.
+ */
+ if (task == kthreadd_task ||
+ (task->flags & PF_NO_SETAFFINITY)) {
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+
ret = task_can_attach(task, cs->cpus_allowed);
if (ret)
goto out_unlock;

2015-11-26 08:06:07

by Chao Peng

Subject: Re: [PATCH] cgroups: move cpuset specific checks from generic code to cpuset_can_attach

On Wed, Nov 25, 2015 at 08:01:17PM -0200, Marcelo Tosatti wrote:
>
> Move PF_NO_SETAFFINITY check to cpuset cgroups, where it belongs.
> This makes it possible to attach PF_NO_SETAFFINITY to Intel CAT cgroups.

Looks that's the right place. I tried intel_rdt subsystem at least it
doesn't have this restriction anymore (all the tasks can be moved out
of the default group), hence:

Reviewed-by: Chao Peng <[email protected]>

Chao

2015-11-26 12:09:32

by Mike Galbraith

Subject: Re: [PATCH] cgroups: move cpuset specific checks from generic code to cpuset_can_attach

On Wed, 2015-11-25 at 20:01 -0200, Marcelo Tosatti wrote:
> Move PF_NO_SETAFFINITY check to cpuset cgroups, where it belongs.

But you took the kthreadd check with it.

-Mike

2015-11-26 22:37:32

by Marcelo Tosatti

Subject: [PATCH] cgroups: move cpuset specific checks from generic code to cpuset_can_attach (v2)

Move PF_NO_SETAFFINITY and kthreadd_task checks to cpuset cgroups,
where they belong. This makes it possible to attach PF_NO_SETAFFINITY
tasks to Intel CAT cgroup.

Reported-by: Luiz Capitulino <[email protected]>
Signed-off-by: Marcelo Tosatti <[email protected]>
Reviewed-by: Chao Peng <[email protected]>

v2: "PF_NO_SETAFFINITY check" -> "PF_NO_SETAFFINITY and kthreadd_task
checks"

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index f89d929..0603652 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -2466,16 +2466,6 @@ static ssize_t __cgroup_procs_write(struct kernfs_open_file *of, char *buf,
if (threadgroup)
tsk = tsk->group_leader;

- /*
- * Workqueue threads may acquire PF_NO_SETAFFINITY and become
- * trapped in a cpuset, or RT worker may be born in a cgroup
- * with no rt_runtime allocated. Just say no.
- */
- if (tsk == kthreadd_task || (tsk->flags & PF_NO_SETAFFINITY)) {
- ret = -EINVAL;
- goto out_unlock_rcu;
- }
-
get_task_struct(tsk);
rcu_read_unlock();

diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index f0acff0..9fff4d6 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -53,6 +53,7 @@
#include <linux/time.h>
#include <linux/backing-dev.h>
#include <linux/sort.h>
+#include <linux/kthread.h>

#include <asm/uaccess.h>
#include <linux/atomic.h>
@@ -1445,6 +1446,17 @@ static int cpuset_can_attach(struct cgroup_subsys_state *css,
goto out_unlock;

cgroup_taskset_for_each(task, tset) {
+ /*
+ * Workqueue threads may acquire PF_NO_SETAFFINITY and become
+ * trapped in a cpuset, or RT worker may be born in a cgroup
+ * with no rt_runtime allocated. Just say no.
+ */
+ if (task == kthreadd_task ||
+ (task->flags & PF_NO_SETAFFINITY)) {
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+
ret = task_can_attach(task, cs->cpus_allowed);
if (ret)
goto out_unlock;

2015-11-26 23:42:20

by Mike Galbraith

Subject: Re: [PATCH] cgroups: move cpuset specific checks from generic code to cpuset_can_attach (v2)

On Thu, 2015-11-26 at 20:32 -0200, Marcelo Tosatti wrote:
> Move PF_NO_SETAFFINITY and kthreadd_task checks to cpuset cgroups,
> where they belong. This makes it possible to attach PF_NO_SETAFFINITY
> tasks to Intel CAT cgroup.
>
> Reported-by: Luiz Capitulino <[email protected]>
> Signed-off-by: Marcelo Tosatti <[email protected]>
> Reviewed-by: Chao Peng <[email protected]>
>
> v2: "PF_NO_SETAFFINITY check" -> "PF_NO_SETAFFINITY and kthreadd_task
> checks"
>
> diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> index f89d929..0603652 100644
> --- a/kernel/cgroup.c
> +++ b/kernel/cgroup.c
> @@ -2466,16 +2466,6 @@ static ssize_t __cgroup_procs_write(struct kernfs_open_file *of, char *buf,
> if (threadgroup)
> tsk = tsk->group_leader;
>
> - /*
> - * Workqueue threads may acquire PF_NO_SETAFFINITY and become
> - * trapped in a cpuset, or RT worker may be born in a cgroup
> - * with no rt_runtime allocated. Just say no.
> - */
> - if (tsk == kthreadd_task || (tsk->flags & PF_NO_SETAFFINITY)) {
> - ret = -EINVAL;
> - goto out_unlock_rcu;
> - }
> -
> get_task_struct(tsk);
> rcu_read_unlock();
>
> diff --git a/kernel/cpuset.c b/kernel/cpuset.c
> index f0acff0..9fff4d6 100644
> --- a/kernel/cpuset.c
> +++ b/kernel/cpuset.c
> @@ -53,6 +53,7 @@
> #include <linux/time.h>
> #include <linux/backing-dev.h>
> #include <linux/sort.h>
> +#include <linux/kthread.h>
>
> #include <asm/uaccess.h>
> #include <linux/atomic.h>
> @@ -1445,6 +1446,17 @@ static int cpuset_can_attach(struct cgroup_subsys_state *css,
> goto out_unlock;
>
> cgroup_taskset_for_each(task, tset) {
> + /*
> + * Workqueue threads may acquire PF_NO_SETAFFINITY and become
> + * trapped in a cpuset, or RT worker may be born in a cgroup
> + * with no rt_runtime allocated. Just say no.
> + */
> + if (task == kthreadd_task ||
> + (task->flags & PF_NO_SETAFFINITY)) {
> + ret = -EINVAL;
> + goto out_unlock;
> + }
> +
> ret = task_can_attach(task, cs->cpus_allowed);
> if (ret)
> goto out_unlock;

Note the rt_runtime reference in the comment, that wasn't about cpuset.
The kthreadd wants to remain immune to all fiddlings methinks.

-Mike

2015-12-01 16:49:21

by Tejun Heo

Subject: Re: [PATCH] cgroups: move cpuset specific checks from generic code to cpuset_can_attach (v2)

On Thu, Nov 26, 2015 at 08:32:32PM -0200, Marcelo Tosatti wrote:
>
> Move PF_NO_SETAFFINITY and kthreadd_task checks to cpuset cgroups,
> where they belong. This makes it possible to attach PF_NO_SETAFFINITY
> tasks to Intel CAT cgroup.
>
> Reported-by: Luiz Capitulino <[email protected]>
> Signed-off-by: Marcelo Tosatti <[email protected]>
> Reviewed-by: Chao Peng <[email protected]>
>
> v2: "PF_NO_SETAFFINITY check" -> "PF_NO_SETAFFINITY and kthreadd_task
> checks"

This wasn't just for cpuset. It's for all controllers.

Thanks.

--
tejun

2015-12-02 06:21:32

by Mike Galbraith

Subject: Re: [PATCH] cgroups: move cpuset specific checks from generic code to cpuset_can_attach (v2)

On Tue, 2015-12-01 at 11:49 -0500, Tejun Heo wrote:
> On Thu, Nov 26, 2015 at 08:32:32PM -0200, Marcelo Tosatti wrote:
> >
> > Move PF_NO_SETAFFINITY and kthreadd_task checks to cpuset cgroups,
> > where they belong. This makes it possible to attach PF_NO_SETAFFINITY
> > tasks to Intel CAT cgroup.
> >
> > Reported-by: Luiz Capitulino <[email protected]>
> > Signed-off-by: Marcelo Tosatti <[email protected]>
> > Reviewed-by: Chao Peng <[email protected]>
> >
> > v2: "PF_NO_SETAFFINITY check" -> "PF_NO_SETAFFINITY and kthreadd_task
> > checks"
>
> This wasn't just for cpuset. It's for all controllers.

Hm, indeed. Workers landing in a throttled cfs rq wouldn't be as
painful as an rt worker being born in an rq with no rt_runtime, but
could sting. Workers stuffed into the freezer would sting mightily.

-Mike