* [PATCH v8 1/7] cgroup: rename freezer.c into legacy_freezer.c
2019-02-19 22:02 [PATCH v8 0/7] freezer for cgroup v2 Roman Gushchin
@ 2019-02-19 22:02 ` Roman Gushchin
2019-02-19 22:02 ` [PATCH v8 2/7] cgroup: implement __cgroup_task_count() helper Roman Gushchin
` (6 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Roman Gushchin @ 2019-02-19 22:02 UTC (permalink / raw)
To: Tejun Heo, Oleg Nesterov
Cc: kernel-team, cgroups, linux-kernel, Roman Gushchin
Freezer.c will contain an implementation of cgroup v2 freezer,
so let's rename the v1 freezer to avoid naming conflicts.
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: kernel-team@fb.com
---
kernel/cgroup/Makefile | 2 +-
kernel/cgroup/{freezer.c => legacy_freezer.c} | 0
2 files changed, 1 insertion(+), 1 deletion(-)
rename kernel/cgroup/{freezer.c => legacy_freezer.c} (100%)
diff --git a/kernel/cgroup/Makefile b/kernel/cgroup/Makefile
index bfcdae896122..8d5689ca94b9 100644
--- a/kernel/cgroup/Makefile
+++ b/kernel/cgroup/Makefile
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
obj-y := cgroup.o rstat.o namespace.o cgroup-v1.o
-obj-$(CONFIG_CGROUP_FREEZER) += freezer.o
+obj-$(CONFIG_CGROUP_FREEZER) += legacy_freezer.o
obj-$(CONFIG_CGROUP_PIDS) += pids.o
obj-$(CONFIG_CGROUP_RDMA) += rdma.o
obj-$(CONFIG_CPUSETS) += cpuset.o
diff --git a/kernel/cgroup/freezer.c b/kernel/cgroup/legacy_freezer.c
similarity index 100%
rename from kernel/cgroup/freezer.c
rename to kernel/cgroup/legacy_freezer.c
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v8 2/7] cgroup: implement __cgroup_task_count() helper
2019-02-19 22:02 [PATCH v8 0/7] freezer for cgroup v2 Roman Gushchin
2019-02-19 22:02 ` [PATCH v8 1/7] cgroup: rename freezer.c into legacy_freezer.c Roman Gushchin
@ 2019-02-19 22:02 ` Roman Gushchin
2019-02-19 22:02 ` [PATCH v8 3/7] cgroup: protect cgroup->nr_(dying_)descendants by css_set_lock Roman Gushchin
` (5 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Roman Gushchin @ 2019-02-19 22:02 UTC (permalink / raw)
To: Tejun Heo, Oleg Nesterov
Cc: kernel-team, cgroups, linux-kernel, Roman Gushchin
The helper is identical to the existing cgroup_task_count()
except it doesn't take the css_set_lock by itself, assuming
that the caller does.
Also, move cgroup_task_count() implementation into
kernel/cgroup/cgroup.c, as there is nothing specific to cgroup v1.
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: kernel-team@fb.com
---
kernel/cgroup/cgroup-internal.h | 1 +
kernel/cgroup/cgroup-v1.c | 16 ----------------
kernel/cgroup/cgroup.c | 33 +++++++++++++++++++++++++++++++++
3 files changed, 34 insertions(+), 16 deletions(-)
diff --git a/kernel/cgroup/cgroup-internal.h b/kernel/cgroup/cgroup-internal.h
index c950864016e2..a195328431ce 100644
--- a/kernel/cgroup/cgroup-internal.h
+++ b/kernel/cgroup/cgroup-internal.h
@@ -226,6 +226,7 @@ int cgroup_rmdir(struct kernfs_node *kn);
int cgroup_show_path(struct seq_file *sf, struct kernfs_node *kf_node,
struct kernfs_root *kf_root);
+int __cgroup_task_count(const struct cgroup *cgrp);
int cgroup_task_count(const struct cgroup *cgrp);
/*
diff --git a/kernel/cgroup/cgroup-v1.c b/kernel/cgroup/cgroup-v1.c
index 583b969b0c0e..29c0ca8f76cf 100644
--- a/kernel/cgroup/cgroup-v1.c
+++ b/kernel/cgroup/cgroup-v1.c
@@ -339,22 +339,6 @@ static struct cgroup_pidlist *cgroup_pidlist_find_create(struct cgroup *cgrp,
return l;
}
-/**
- * cgroup_task_count - count the number of tasks in a cgroup.
- * @cgrp: the cgroup in question
- */
-int cgroup_task_count(const struct cgroup *cgrp)
-{
- int count = 0;
- struct cgrp_cset_link *link;
-
- spin_lock_irq(&css_set_lock);
- list_for_each_entry(link, &cgrp->cset_links, cset_link)
- count += link->cset->nr_tasks;
- spin_unlock_irq(&css_set_lock);
- return count;
-}
-
/*
* Load a cgroup's pidarray with either procs' tgids or tasks' pids
*/
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index f4418371c83b..b230afccf635 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -592,6 +592,39 @@ static void cgroup_get_live(struct cgroup *cgrp)
css_get(&cgrp->self);
}
+/**
+ * __cgroup_task_count - count the number of tasks in a cgroup. The caller
+ * is responsible for taking the css_set_lock.
+ * @cgrp: the cgroup in question
+ */
+int __cgroup_task_count(const struct cgroup *cgrp)
+{
+ int count = 0;
+ struct cgrp_cset_link *link;
+
+ lockdep_assert_held(&css_set_lock);
+
+ list_for_each_entry(link, &cgrp->cset_links, cset_link)
+ count += link->cset->nr_tasks;
+
+ return count;
+}
+
+/**
+ * cgroup_task_count - count the number of tasks in a cgroup.
+ * @cgrp: the cgroup in question
+ */
+int cgroup_task_count(const struct cgroup *cgrp)
+{
+ int count;
+
+ spin_lock_irq(&css_set_lock);
+ count = __cgroup_task_count(cgrp);
+ spin_unlock_irq(&css_set_lock);
+
+ return count;
+}
+
struct cgroup_subsys_state *of_css(struct kernfs_open_file *of)
{
struct cgroup *cgrp = of->kn->parent->priv;
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v8 3/7] cgroup: protect cgroup->nr_(dying_)descendants by css_set_lock
2019-02-19 22:02 [PATCH v8 0/7] freezer for cgroup v2 Roman Gushchin
2019-02-19 22:02 ` [PATCH v8 1/7] cgroup: rename freezer.c into legacy_freezer.c Roman Gushchin
2019-02-19 22:02 ` [PATCH v8 2/7] cgroup: implement __cgroup_task_count() helper Roman Gushchin
@ 2019-02-19 22:02 ` Roman Gushchin
2019-02-19 22:02 ` [PATCH v8 4/7] cgroup: cgroup v2 freezer Roman Gushchin
` (4 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Roman Gushchin @ 2019-02-19 22:02 UTC (permalink / raw)
To: Tejun Heo, Oleg Nesterov
Cc: kernel-team, cgroups, linux-kernel, Roman Gushchin
The number of descendant cgroups and the number of dying
descendant cgroups are currently synchronized using the cgroup_mutex.
The number of descendant cgroups will be required by the cgroup v2
freezer, which will use it to determine if a cgroup is frozen
(depending on total number of descendants and number of frozen
descendants). It's not always acceptable to grab the cgroup_mutex,
especially from quite hot paths (e.g. exit()).
To avoid this, let's additionally synchronize these counters using
the css_set_lock.
So, it's safe to read these counters with either cgroup_mutex or
css_set_lock locked, and for changing both locks should be acquired.
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: kernel-team@fb.com
---
include/linux/cgroup-defs.h | 5 +++++
kernel/cgroup/cgroup.c | 6 ++++++
2 files changed, 11 insertions(+)
diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index 120d1d40704b..319c07305500 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -348,6 +348,11 @@ struct cgroup {
* Dying cgroups are cgroups which were deleted by a user,
* but are still existing because someone else is holding a reference.
* max_descendants is a maximum allowed number of descent cgroups.
+ *
+ * nr_descendants and nr_dying_descendants are protected
+ * by cgroup_mutex and css_set_lock. It's fine to read them holding
+ * any of cgroup_mutex and css_set_lock; for writing both locks
+ * should be held.
*/
int nr_descendants;
int nr_dying_descendants;
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index b230afccf635..7438c24297d4 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -4758,9 +4758,11 @@ static void css_release_work_fn(struct work_struct *work)
if (cgroup_on_dfl(cgrp))
cgroup_rstat_flush(cgrp);
+ spin_lock_irq(&css_set_lock);
for (tcgrp = cgroup_parent(cgrp); tcgrp;
tcgrp = cgroup_parent(tcgrp))
tcgrp->nr_dying_descendants--;
+ spin_unlock_irq(&css_set_lock);
cgroup_idr_remove(&cgrp->root->cgroup_idr, cgrp->id);
cgrp->id = -1;
@@ -4978,12 +4980,14 @@ static struct cgroup *cgroup_create(struct cgroup *parent)
if (ret)
goto out_psi_free;
+ spin_lock_irq(&css_set_lock);
for (tcgrp = cgrp; tcgrp; tcgrp = cgroup_parent(tcgrp)) {
cgrp->ancestor_ids[tcgrp->level] = tcgrp->id;
if (tcgrp != cgrp)
tcgrp->nr_descendants++;
}
+ spin_unlock_irq(&css_set_lock);
if (notify_on_release(parent))
set_bit(CGRP_NOTIFY_ON_RELEASE, &cgrp->flags);
@@ -5268,10 +5272,12 @@ static int cgroup_destroy_locked(struct cgroup *cgrp)
if (parent && cgroup_is_threaded(cgrp))
parent->nr_threaded_children--;
+ spin_lock_irq(&css_set_lock);
for (tcgrp = cgroup_parent(cgrp); tcgrp; tcgrp = cgroup_parent(tcgrp)) {
tcgrp->nr_descendants--;
tcgrp->nr_dying_descendants++;
}
+ spin_unlock_irq(&css_set_lock);
cgroup1_check_for_release(parent);
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v8 4/7] cgroup: cgroup v2 freezer
2019-02-19 22:02 [PATCH v8 0/7] freezer for cgroup v2 Roman Gushchin
` (2 preceding siblings ...)
2019-02-19 22:02 ` [PATCH v8 3/7] cgroup: protect cgroup->nr_(dying_)descendants by css_set_lock Roman Gushchin
@ 2019-02-19 22:02 ` Roman Gushchin
2019-02-20 14:42 ` Oleg Nesterov
2019-02-19 22:02 ` [PATCH v8 5/7] kselftests: cgroup: don't fail on cg_kill_all() error in cg_destroy() Roman Gushchin
` (3 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Roman Gushchin @ 2019-02-19 22:02 UTC (permalink / raw)
To: Tejun Heo, Oleg Nesterov
Cc: kernel-team, cgroups, linux-kernel, Roman Gushchin
Cgroup v1 implements the freezer controller, which provides an ability
to stop the workload in a cgroup and temporarily free up some
resources (cpu, io, network bandwidth and, potentially, memory)
for some other tasks. Cgroup v2 lacks this functionality.
This patch implements freezer for cgroup v2.
Cgroup v2 freezer tries to put tasks into a state similar to jobctl
stop. This means that tasks can be killed, ptraced (using
PTRACE_SEIZE*), and interrupted. It is possible to attach to
a frozen task, get some information (e.g. read registers) and detach.
It's also possible to migrate a frozen tasks to another cgroup.
This differs cgroup v2 freezer from cgroup v1 freezer, which mostly
tried to imitate the system-wide freezer. However uninterruptible
sleep is fine when all tasks are going to be frozen (hibernation case),
it's not the acceptable state for some subset of the system.
Cgroup v2 freezer is not supporting freezing kthreads.
If a non-root cgroup contains kthread, the cgroup still can be frozen,
but the kthread will remain running, the cgroup will be shown
as non-frozen, and the notification will not be delivered.
* PTRACE_ATTACH is not working because non-fatal signal delivery
is blocked in frozen state.
There are some interface differences between cgroup v1 and cgroup v2
freezer too, which are required to conform the cgroup v2 interface
design principles:
1) There is no separate controller, which has to be turned on:
the functionality is always available and is represented by
cgroup.freeze and cgroup.events cgroup control files.
2) The desired state is defined by the cgroup.freeze control file.
Any hierarchical configuration is allowed.
3) The interface is asynchronous. The actual state is available
using cgroup.events control file ("frozen" field). There are no
dedicated transitional states.
4) It's allowed to make any changes with the cgroup hierarchy
(create new cgroups, remove old cgroups, move tasks between cgroups)
no matter if some cgroups are frozen.
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: kernel-team@fb.com
---
include/linux/cgroup-defs.h | 28 ++++
include/linux/cgroup.h | 42 ++++++
include/linux/sched.h | 2 +
include/linux/sched/jobctl.h | 2 +
kernel/cgroup/Makefile | 2 +-
kernel/cgroup/cgroup.c | 103 ++++++++++++-
kernel/cgroup/freezer.c | 281 +++++++++++++++++++++++++++++++++++
kernel/signal.c | 84 ++++++++++-
8 files changed, 533 insertions(+), 11 deletions(-)
create mode 100644 kernel/cgroup/freezer.c
diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index 319c07305500..29c94850cfa7 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -64,6 +64,12 @@ enum {
* specified at mount time and thus is implemented here.
*/
CGRP_CPUSET_CLONE_CHILDREN,
+
+ /* Control group has to be frozen. */
+ CGRP_FREEZE,
+
+ /* Cgroup is frozen. */
+ CGRP_FROZEN,
};
/* cgroup_root->flags */
@@ -316,6 +322,25 @@ struct cgroup_rstat_cpu {
struct cgroup *updated_next; /* NULL iff not on the list */
};
+struct cgroup_freezer_state {
+ /* Should the cgroup and its descendants be frozen. */
+ bool freeze;
+
+ /* Should the cgroup actually be frozen? */
+ int e_freeze;
+
+ /* Fields below are protected by css_set_lock */
+
+ /* Number of frozen descendant cgroups */
+ int nr_frozen_descendants;
+
+ /*
+ * Number of tasks, which are counted as frozen:
+ * frozen, SIGSTOPped, and PTRACEd.
+ */
+ int nr_stopped_tasks;
+};
+
struct cgroup {
/* self css with NULL ->ss, points back to this cgroup */
struct cgroup_subsys_state self;
@@ -452,6 +477,9 @@ struct cgroup {
/* If there is block congestion on this cgroup. */
atomic_t congestion_count;
+ /* Used to store internal freezer state */
+ struct cgroup_freezer_state freezer;
+
/* ids of the ancestors at each level including self */
int ancestor_ids[];
};
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 81f58b4a5418..d748db6ccd9b 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -881,4 +881,46 @@ static inline void put_cgroup_ns(struct cgroup_namespace *ns)
free_cgroup_ns(ns);
}
+#ifdef CONFIG_CGROUPS
+
+void cgroup_enter_stopped(void);
+void cgroup_leave_stopped(void);
+void cgroup_update_frozen(struct cgroup *cgrp);
+void cgroup_freeze(struct cgroup *cgrp, bool freeze);
+void cgroup_freezer_migrate_task(struct task_struct *task, struct cgroup *src,
+ struct cgroup *dst);
+static inline bool cgroup_task_freeze(struct task_struct *task)
+{
+ bool ret;
+
+ if (task->flags & PF_KTHREAD)
+ return false;
+
+ rcu_read_lock();
+ ret = test_bit(CGRP_FREEZE, &task_dfl_cgroup(task)->flags);
+ rcu_read_unlock();
+
+ return ret;
+}
+
+static inline bool cgroup_task_stopped(struct task_struct *task)
+{
+ return task->stopped;
+}
+
+#else /* !CONFIG_CGROUPS */
+
+static inline void cgroup_enter_stopped(void) { }
+static inline void cgroup_leave_stopped(void) { }
+static inline bool cgroup_task_freeze(struct task_struct *task)
+{
+ return false;
+}
+static inline bool cgroup_task_stopped(struct task_struct *task)
+{
+ return false;
+}
+
+#endif /* !CONFIG_CGROUPS */
+
#endif /* _LINUX_CGROUP_H */
diff --git a/include/linux/sched.h b/include/linux/sched.h
index d2f90fa92468..825da92d0280 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -733,6 +733,8 @@ struct task_struct {
#ifdef CONFIG_CGROUPS
/* disallow userland-initiated cgroup migration */
unsigned no_cgroup_migration:1;
+ /* task is frozen/stopped (used by the cgroup freezer) */
+ unsigned stopped:1;
#endif
#ifdef CONFIG_BLK_CGROUP
/* to be used once the psi infrastructure lands upstream. */
diff --git a/include/linux/sched/jobctl.h b/include/linux/sched/jobctl.h
index 98228bd48aee..fa067de9f1a9 100644
--- a/include/linux/sched/jobctl.h
+++ b/include/linux/sched/jobctl.h
@@ -18,6 +18,7 @@ struct task_struct;
#define JOBCTL_TRAP_NOTIFY_BIT 20 /* trap for NOTIFY */
#define JOBCTL_TRAPPING_BIT 21 /* switching to TRACED */
#define JOBCTL_LISTENING_BIT 22 /* ptracer is listening for events */
+#define JOBCTL_TRAP_FREEZE_BIT 23 /* trap for cgroup freezer */
#define JOBCTL_STOP_DEQUEUED (1UL << JOBCTL_STOP_DEQUEUED_BIT)
#define JOBCTL_STOP_PENDING (1UL << JOBCTL_STOP_PENDING_BIT)
@@ -26,6 +27,7 @@ struct task_struct;
#define JOBCTL_TRAP_NOTIFY (1UL << JOBCTL_TRAP_NOTIFY_BIT)
#define JOBCTL_TRAPPING (1UL << JOBCTL_TRAPPING_BIT)
#define JOBCTL_LISTENING (1UL << JOBCTL_LISTENING_BIT)
+#define JOBCTL_TRAP_FREEZE (1UL << JOBCTL_TRAP_FREEZE_BIT)
#define JOBCTL_TRAP_MASK (JOBCTL_TRAP_STOP | JOBCTL_TRAP_NOTIFY)
#define JOBCTL_PENDING_MASK (JOBCTL_STOP_PENDING | JOBCTL_TRAP_MASK)
diff --git a/kernel/cgroup/Makefile b/kernel/cgroup/Makefile
index 8d5689ca94b9..5d7a76bfbbb7 100644
--- a/kernel/cgroup/Makefile
+++ b/kernel/cgroup/Makefile
@@ -1,5 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
-obj-y := cgroup.o rstat.o namespace.o cgroup-v1.o
+obj-y := cgroup.o rstat.o namespace.o cgroup-v1.o freezer.o
obj-$(CONFIG_CGROUP_FREEZER) += legacy_freezer.o
obj-$(CONFIG_CGROUP_PIDS) += pids.o
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 7438c24297d4..3b8883dcf9c3 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -2391,8 +2391,15 @@ static int cgroup_migrate_execute(struct cgroup_mgctx *mgctx)
get_css_set(to_cset);
to_cset->nr_tasks++;
css_set_move_task(task, from_cset, to_cset, true);
- put_css_set_locked(from_cset);
from_cset->nr_tasks--;
+ /*
+ * If the source or destination cgroup is frozen,
+ * the task might require to change its state.
+ */
+ cgroup_freezer_migrate_task(task, from_cset->dfl_cgrp,
+ to_cset->dfl_cgrp);
+ put_css_set_locked(from_cset);
+
}
}
spin_unlock_irq(&css_set_lock);
@@ -3436,8 +3443,11 @@ static ssize_t cgroup_max_depth_write(struct kernfs_open_file *of,
static int cgroup_events_show(struct seq_file *seq, void *v)
{
- seq_printf(seq, "populated %d\n",
- cgroup_is_populated(seq_css(seq)->cgroup));
+ struct cgroup *cgrp = seq_css(seq)->cgroup;
+
+ seq_printf(seq, "populated %d\n", cgroup_is_populated(cgrp));
+ seq_printf(seq, "frozen %d\n", test_bit(CGRP_FROZEN, &cgrp->flags));
+
return 0;
}
@@ -3499,6 +3509,40 @@ static int cgroup_cpu_pressure_show(struct seq_file *seq, void *v)
}
#endif
+static int cgroup_freeze_show(struct seq_file *seq, void *v)
+{
+ struct cgroup *cgrp = seq_css(seq)->cgroup;
+
+ seq_printf(seq, "%d\n", cgrp->freezer.freeze);
+
+ return 0;
+}
+
+static ssize_t cgroup_freeze_write(struct kernfs_open_file *of,
+ char *buf, size_t nbytes, loff_t off)
+{
+ struct cgroup *cgrp;
+ ssize_t ret;
+ int freeze;
+
+ ret = kstrtoint(strstrip(buf), 0, &freeze);
+ if (ret)
+ return ret;
+
+ if (freeze < 0 || freeze > 1)
+ return -ERANGE;
+
+ cgrp = cgroup_kn_lock_live(of->kn, false);
+ if (!cgrp)
+ return -ENOENT;
+
+ cgroup_freeze(cgrp, freeze);
+
+ cgroup_kn_unlock(of->kn);
+
+ return nbytes;
+}
+
static int cgroup_file_open(struct kernfs_open_file *of)
{
struct cftype *cft = of->kn->priv;
@@ -4630,6 +4674,12 @@ static struct cftype cgroup_base_files[] = {
.name = "cgroup.stat",
.seq_show = cgroup_stat_show,
},
+ {
+ .name = "cgroup.freeze",
+ .flags = CFTYPE_NOT_ON_ROOT,
+ .seq_show = cgroup_freeze_show,
+ .write = cgroup_freeze_write,
+ },
{
.name = "cpu.stat",
.flags = CFTYPE_NOT_ON_ROOT,
@@ -4980,12 +5030,29 @@ static struct cgroup *cgroup_create(struct cgroup *parent)
if (ret)
goto out_psi_free;
+ /*
+ * New cgroup inherits effective freeze counter, and
+ * if the parent has to be frozen, the child has too.
+ */
+ cgrp->freezer.e_freeze = parent->freezer.e_freeze;
+ if (cgrp->freezer.e_freeze)
+ set_bit(CGRP_FROZEN, &cgrp->flags);
+
spin_lock_irq(&css_set_lock);
for (tcgrp = cgrp; tcgrp; tcgrp = cgroup_parent(tcgrp)) {
cgrp->ancestor_ids[tcgrp->level] = tcgrp->id;
- if (tcgrp != cgrp)
+ if (tcgrp != cgrp) {
tcgrp->nr_descendants++;
+
+ /*
+ * If the new cgroup is frozen, all ancestor cgroups
+ * get a new frozen descendant, but their state can't
+ * change because of this.
+ */
+ if (cgrp->freezer.e_freeze)
+ tcgrp->freezer.nr_frozen_descendants++;
+ }
}
spin_unlock_irq(&css_set_lock);
@@ -5276,6 +5343,12 @@ static int cgroup_destroy_locked(struct cgroup *cgrp)
for (tcgrp = cgroup_parent(cgrp); tcgrp; tcgrp = cgroup_parent(tcgrp)) {
tcgrp->nr_descendants--;
tcgrp->nr_dying_descendants++;
+ /*
+ * If the dying cgroup is frozen, decrease frozen descendants
+ * counters of ancestor cgroups.
+ */
+ if (test_bit(CGRP_FROZEN, &cgrp->flags))
+ tcgrp->freezer.nr_frozen_descendants--;
}
spin_unlock_irq(&css_set_lock);
@@ -5729,6 +5802,24 @@ void cgroup_post_fork(struct task_struct *child)
cset->nr_tasks++;
css_set_move_task(child, NULL, cset, false);
}
+
+ /*
+ * If the cgroup has to be frozen, the new task has too.
+ * Let's update cgroup freezer counter and set the
+ * JOBCTL_TRAP_FREEZE jobctl bit to get the tack into
+ * the frozen state.
+ */
+ if (unlikely(cgroup_task_freeze(child))) {
+ struct cgroup *cgrp;
+
+ spin_lock(&child->sighand->siglock);
+ cgrp = cset->dfl_cgrp;
+ child->jobctl |= JOBCTL_TRAP_FREEZE;
+ spin_unlock(&child->sighand->siglock);
+
+ cgroup_update_frozen(cgrp);
+ }
+
spin_unlock_irq(&css_set_lock);
}
@@ -5777,6 +5868,10 @@ void cgroup_exit(struct task_struct *tsk)
spin_lock_irq(&css_set_lock);
css_set_move_task(tsk, cset, NULL, false);
cset->nr_tasks--;
+
+ if (unlikely(cgroup_task_freeze(tsk)))
+ cgroup_update_frozen(task_dfl_cgroup(tsk));
+
spin_unlock_irq(&css_set_lock);
} else {
get_css_set(cset);
diff --git a/kernel/cgroup/freezer.c b/kernel/cgroup/freezer.c
new file mode 100644
index 000000000000..5ca7db4205af
--- /dev/null
+++ b/kernel/cgroup/freezer.c
@@ -0,0 +1,281 @@
+//SPDX-License-Identifier: GPL-2.0
+#include <linux/cgroup.h>
+#include <linux/sched.h>
+#include <linux/sched/task.h>
+#include <linux/sched/signal.h>
+
+#include "cgroup-internal.h"
+
+/*
+ * Propagate the cgroup frozen state upwards by the cgroup tree.
+ */
+static void cgroup_propagate_frozen(struct cgroup *cgrp, bool frozen)
+{
+ int desc = 1;
+
+ /*
+ * If the new state is frozen, some freezing ancestor cgroups may change
+ * their state too, depending on if all their descendants are frozen.
+ *
+ * Otherwise, all ancestor cgroups are forced into the non-frozen state.
+ */
+ while ((cgrp = cgroup_parent(cgrp))) {
+ if (frozen) {
+ cgrp->freezer.nr_frozen_descendants += desc;
+ if (!test_bit(CGRP_FROZEN, &cgrp->flags) &&
+ test_bit(CGRP_FREEZE, &cgrp->flags) &&
+ cgrp->freezer.nr_frozen_descendants ==
+ cgrp->nr_descendants) {
+ set_bit(CGRP_FROZEN, &cgrp->flags);
+ cgroup_file_notify(&cgrp->events_file);
+ desc++;
+ }
+ } else {
+ cgrp->freezer.nr_frozen_descendants -= desc;
+ if (test_bit(CGRP_FROZEN, &cgrp->flags)) {
+ clear_bit(CGRP_FROZEN, &cgrp->flags);
+ cgroup_file_notify(&cgrp->events_file);
+ desc++;
+ }
+ }
+ }
+}
+
+/*
+ * Revisit the cgroup frozen state.
+ * Checks if the cgroup is really frozen and perform all state transitions.
+ */
+void cgroup_update_frozen(struct cgroup *cgrp)
+{
+ bool frozen;
+
+ lockdep_assert_held(&css_set_lock);
+
+ /*
+ * If the cgroup has to be frozen (CGRP_FREEZE bit set),
+ * and all tasks are frozen and/or stopped, let's consider
+ * the cgroup frozen. Otherwise it's not frozen.
+ */
+ frozen = test_bit(CGRP_FREEZE, &cgrp->flags) &&
+ cgrp->freezer.nr_stopped_tasks == __cgroup_task_count(cgrp);
+
+ if (frozen) {
+ /* Already there? */
+ if (test_bit(CGRP_FROZEN, &cgrp->flags))
+ return;
+
+ set_bit(CGRP_FROZEN, &cgrp->flags);
+ } else {
+ /* Already there? */
+ if (!test_bit(CGRP_FROZEN, &cgrp->flags))
+ return;
+
+ clear_bit(CGRP_FROZEN, &cgrp->flags);
+ }
+ cgroup_file_notify(&cgrp->events_file);
+
+ /* Update the state of ancestor cgroups. */
+ cgroup_propagate_frozen(cgrp, frozen);
+}
+
+/*
+ * Increment cgroup's nr_stopped_tasks and revisit cgroup state if necessary.
+ */
+static void cgroup_inc_frozen_cnt(struct cgroup *cgrp)
+{
+ cgrp->freezer.nr_stopped_tasks++;
+ cgroup_update_frozen(cgrp);
+}
+
+/*
+ * Decrement cgroup's nr_stopped_tasks and revisit cgroup state if necessary.
+ */
+static void cgroup_dec_frozen_cnt(struct cgroup *cgrp)
+{
+ cgrp->freezer.nr_stopped_tasks--;
+ WARN_ON_ONCE(cgrp->freezer.nr_stopped_tasks < 0);
+ cgroup_update_frozen(cgrp);
+}
+
+/*
+ * Enter frozen/stopped state, if not yet there. Update cgroup's counters,
+ * and revisit the state of the cgroup, if necessary.
+ */
+void cgroup_enter_stopped(void)
+{
+ if (!current->stopped) {
+ spin_lock_irq(&css_set_lock);
+ current->stopped = true;
+ cgroup_inc_frozen_cnt(task_dfl_cgroup(current));
+ spin_unlock_irq(&css_set_lock);
+ }
+}
+
+/*
+ * Conditionally leave frozen/stopped state. Update cgroup's counters,
+ * and revisit the state of the cgroup, if necessary.
+ */
+void cgroup_leave_stopped(void)
+{
+ if (current->stopped) {
+ spin_lock_irq(&css_set_lock);
+ cgroup_dec_frozen_cnt(task_dfl_cgroup(current));
+ current->stopped = false;
+ spin_unlock_irq(&css_set_lock);
+ }
+}
+
+/*
+ * Freeze or unfreeze the task by setting or clearing the JOBCTL_TRAP_FREEZE
+ * jobctl bit.
+ */
+static void cgroup_freeze_task(struct task_struct *task, bool freeze)
+{
+ unsigned long flags;
+
+ /* If the task is about to die, don't bother with freezing it. */
+ if (!lock_task_sighand(task, &flags))
+ return;
+
+ if (freeze) {
+ task->jobctl |= JOBCTL_TRAP_FREEZE;
+ signal_wake_up(task, false);
+ } else {
+ task->jobctl &= ~JOBCTL_TRAP_FREEZE;
+ wake_up_process(task);
+ }
+
+ unlock_task_sighand(task, &flags);
+}
+
+/*
+ * Freeze or unfreeze all tasks in the given cgroup.
+ */
+static void cgroup_do_freeze(struct cgroup *cgrp, bool freeze)
+{
+ struct css_task_iter it;
+ struct task_struct *task;
+
+ lockdep_assert_held(&cgroup_mutex);
+
+ spin_lock_irq(&css_set_lock);
+ if (freeze) {
+ set_bit(CGRP_FREEZE, &cgrp->flags);
+ } else {
+ clear_bit(CGRP_FREEZE, &cgrp->flags);
+ }
+ spin_unlock_irq(&css_set_lock);
+
+ css_task_iter_start(&cgrp->self, 0, &it);
+ while ((task = css_task_iter_next(&it))) {
+ /*
+ * Ignore kernel threads here. Freezing cgroups containing
+ * kthreads isn't supported.
+ */
+ if (task->flags & PF_KTHREAD)
+ continue;
+ cgroup_freeze_task(task, freeze);
+ }
+ css_task_iter_end(&it);
+
+ /*
+ * Cgroup state should be revisited here to cover empty leaf cgroups
+ * and cgroups which descendants are already in the desired state.
+ */
+ spin_lock_irq(&css_set_lock);
+ if (cgrp->nr_descendants == cgrp->freezer.nr_frozen_descendants)
+ cgroup_update_frozen(cgrp);
+ spin_unlock_irq(&css_set_lock);
+}
+
+void cgroup_freezer_migrate_task(struct task_struct *task,
+ struct cgroup *src, struct cgroup *dst)
+{
+ lockdep_assert_held(&css_set_lock);
+
+ /*
+ * Kernel threads are not supposed to be frozen at all.
+ */
+ if (task->flags & PF_KTHREAD)
+ return;
+
+ /*
+ * Adjust counters of freezing and frozen tasks.
+ * Note, that if the task is frozen, but the destination cgroup is not
+ * frozen, we bump both counters to keep them balanced.
+ */
+ if (task->stopped) {
+ cgroup_dec_frozen_cnt(src);
+ cgroup_inc_frozen_cnt(dst);
+ } else {
+ cgroup_update_frozen(src);
+ cgroup_update_frozen(dst);
+ }
+
+ /*
+ * Force the task to the desired state.
+ */
+ cgroup_freeze_task(task, test_bit(CGRP_FREEZE, &dst->flags));
+}
+
+void cgroup_freeze(struct cgroup *cgrp, bool freeze)
+{
+ struct cgroup_subsys_state *css;
+ struct cgroup *dsct;
+ bool applied = false;
+
+ lockdep_assert_held(&cgroup_mutex);
+
+ /*
+ * Nothing changed? Just exit.
+ */
+ if (cgrp->freezer.freeze == freeze)
+ return;
+
+ cgrp->freezer.freeze = freeze;
+
+ /*
+ * Propagate changes downwards the cgroup tree.
+ */
+ css_for_each_descendant_pre(css, &cgrp->self) {
+ dsct = css->cgroup;
+
+ if (cgroup_is_dead(dsct))
+ continue;
+
+ if (freeze) {
+ dsct->freezer.e_freeze++;
+ /*
+ * Already frozen because of ancestor's settings?
+ */
+ if (dsct->freezer.e_freeze > 1)
+ continue;
+ } else {
+ dsct->freezer.e_freeze--;
+ /*
+ * Still frozen because of ancestor's settings?
+ */
+ if (dsct->freezer.e_freeze > 0)
+ continue;
+
+ WARN_ON_ONCE(dsct->freezer.e_freeze < 0);
+ }
+
+ /*
+ * Do change actual state: freeze or unfreeze.
+ */
+ cgroup_do_freeze(dsct, freeze);
+ applied = true;
+ }
+
+ /*
+ * Even if the actual state hasn't changed, let's notify a user.
+ * The state can be enforced by an ancestor cgroup: the cgroup
+ * can already be in the desired state or it can be locked in the
+ * opposite state, so that the transition will never happen.
+ * In both cases it's better to notify a user, that there is
+ * nothing to wait.
+ */
+ if (!applied)
+ cgroup_file_notify(&cgrp->events_file);
+}
diff --git a/kernel/signal.c b/kernel/signal.c
index e1d7ad8e6ab1..82e7e82bb527 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -41,6 +41,7 @@
#include <linux/compiler.h>
#include <linux/posix-timers.h>
#include <linux/livepatch.h>
+#include <linux/cgroup.h>
#define CREATE_TRACE_POINTS
#include <trace/events/signal.h>
@@ -144,7 +145,7 @@ static inline bool has_pending_signals(sigset_t *signal, sigset_t *blocked)
static bool recalc_sigpending_tsk(struct task_struct *t)
{
- if ((t->jobctl & JOBCTL_PENDING_MASK) ||
+ if ((t->jobctl & (JOBCTL_PENDING_MASK | JOBCTL_TRAP_FREEZE)) ||
PENDING(&t->pending, &t->blocked) ||
PENDING(&t->signal->shared_pending, &t->blocked)) {
set_tsk_thread_flag(t, TIF_SIGPENDING);
@@ -2065,6 +2066,7 @@ static void ptrace_stop(int exit_code, int why, int clear_code, kernel_siginfo_t
preempt_disable();
read_unlock(&tasklist_lock);
preempt_enable_no_resched();
+ cgroup_enter_stopped();
freezable_schedule();
} else {
/*
@@ -2243,6 +2245,7 @@ static bool do_signal_stop(int signr)
}
/* Now we don't run again until woken by SIGCONT or SIGKILL */
+ cgroup_enter_stopped();
freezable_schedule();
return true;
} else {
@@ -2266,6 +2269,9 @@ static bool do_signal_stop(int signr)
* When !PT_SEIZED, it's used only for group stop trap with stop signal
* number as exit_code and no siginfo.
*
+ * When used with JOBCTL_TRAP_FREEZE, it puts the task into an interruptible
+ * sleep if only no fatal signals are pending.
+ *
* CONTEXT:
* Must be called with @current->sighand->siglock held, which may be
* released and re-acquired before returning with intervening sleep.
@@ -2280,7 +2286,8 @@ static void do_jobctl_trap(void)
!(signal->flags & SIGNAL_STOP_STOPPED))
signr = SIGTRAP;
WARN_ON_ONCE(!signr);
- ptrace_do_notify(signr, signr | (PTRACE_EVENT_STOP << 8),
+ ptrace_do_notify(signr,
+ signr | (PTRACE_EVENT_STOP << 8),
CLD_STOPPED);
} else {
WARN_ON_ONCE(!signr);
@@ -2289,6 +2296,55 @@ static void do_jobctl_trap(void)
}
}
+/**
+ * do_freezer_trap - handle the freezer jobctl trap
+ *
+ * Puts the task into frozen state, if only the task is not about to quit.
+ * In this case it drops JOBCTL_TRAP_FREEZE.
+ *
+ * CONTEXT:
+ * Must be called with @current->sighand->siglock held,
+ * which is always released before returning.
+ */
+static void do_freezer_trap(void)
+ __releases(¤t->sighand->siglock)
+{
+ /*
+ * If a fatal signal is pending, there is no way back for the process,
+ * so let it escape from the freezer trap and exit.
+ * If the task has been frozen, cgroup_leave_frozen() will be invoked
+ * to update the cgroup state, if necessary.
+ */
+ if (fatal_signal_pending(current)) {
+ current->jobctl &= ~JOBCTL_TRAP_FREEZE;
+ spin_unlock_irq(¤t->sighand->siglock);
+ return;
+ }
+
+ /*
+ * If there are other trap bits pending except JOBCTL_TRAP_FREEZE,
+ * let's make another loop to give it a chance to be handled.
+ * In any case, we'll return back.
+ */
+ if (((current->jobctl & (JOBCTL_PENDING_MASK | JOBCTL_TRAP_FREEZE)) !=
+ JOBCTL_TRAP_FREEZE) || fatal_signal_pending(current)) {
+ spin_unlock_irq(¤t->sighand->siglock);
+ return;
+ }
+
+ /*
+ * Now we're sure that there is no pending fatal signal and no
+ * pending traps. Clear TIF_SIGPENDING to not get out of schedule()
+ * immediately (if there is a non-fatal signal pending), and
+ * put the task into sleep.
+ */
+ __set_current_state(TASK_INTERRUPTIBLE);
+ clear_thread_flag(TIF_SIGPENDING);
+ spin_unlock_irq(¤t->sighand->siglock);
+ cgroup_enter_stopped();
+ freezable_schedule();
+}
+
static int ptrace_signal(int signr, kernel_siginfo_t *info)
{
/*
@@ -2363,7 +2419,8 @@ bool get_signal(struct ksignal *ksig)
* we should notify the parent, prepare_signal(SIGCONT) encodes
* the CLD_ si_code into SIGNAL_CLD_MASK bits.
*/
- if (unlikely(signal->flags & SIGNAL_CLD_MASK)) {
+ if (unlikely(signal->flags & SIGNAL_CLD_MASK) &
+ !(current->jobctl & JOBCTL_TRAP_FREEZE)) {
int why;
if (signal->flags & SIGNAL_CLD_CONTINUED)
@@ -2401,12 +2458,27 @@ bool get_signal(struct ksignal *ksig)
do_signal_stop(0))
goto relock;
- if (unlikely(current->jobctl & JOBCTL_TRAP_MASK)) {
- do_jobctl_trap();
- spin_unlock_irq(&sighand->siglock);
+ if (unlikely(current->jobctl &
+ (JOBCTL_TRAP_MASK | JOBCTL_TRAP_FREEZE))) {
+ if (current->jobctl & JOBCTL_TRAP_MASK) {
+ do_jobctl_trap();
+ spin_unlock_irq(&sighand->siglock);
+ } else if (current->jobctl & JOBCTL_TRAP_FREEZE)
+ do_freezer_trap();
+
goto relock;
}
+ /*
+ * If the task is leaving the frozen state, let's update
+ * cgroup counters and reset the frozen bit.
+ */
+ if (unlikely(cgroup_task_stopped(current))) {
+ spin_unlock_irq(&sighand->siglock);
+ cgroup_leave_stopped();
+ spin_lock_irq(&sighand->siglock);
+ }
+
signr = dequeue_signal(current, ¤t->blocked, &ksig->info);
if (!signr)
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v8 4/7] cgroup: cgroup v2 freezer
2019-02-19 22:02 ` [PATCH v8 4/7] cgroup: cgroup v2 freezer Roman Gushchin
@ 2019-02-20 14:42 ` Oleg Nesterov
2019-02-20 22:14 ` Roman Gushchin
0 siblings, 1 reply; 21+ messages in thread
From: Oleg Nesterov @ 2019-02-20 14:42 UTC (permalink / raw)
To: Roman Gushchin
Cc: Tejun Heo, kernel-team, cgroups, linux-kernel, Roman Gushchin
On 02/19, Roman Gushchin wrote:
>
> @@ -2065,6 +2066,7 @@ static void ptrace_stop(int exit_code, int why, int clear_code, kernel_siginfo_t
> preempt_disable();
> read_unlock(&tasklist_lock);
> preempt_enable_no_resched();
> + cgroup_enter_stopped();
> freezable_schedule();
> } else {
> /*
> @@ -2243,6 +2245,7 @@ static bool do_signal_stop(int signr)
> }
>
> /* Now we don't run again until woken by SIGCONT or SIGKILL */
> + cgroup_enter_stopped();
> freezable_schedule();
> return true;
Please see my reply to 0/7.
> @@ -2280,7 +2286,8 @@ static void do_jobctl_trap(void)
> !(signal->flags & SIGNAL_STOP_STOPPED))
> signr = SIGTRAP;
> WARN_ON_ONCE(!signr);
> - ptrace_do_notify(signr, signr | (PTRACE_EVENT_STOP << 8),
> + ptrace_do_notify(signr,
> + signr | (PTRACE_EVENT_STOP << 8),
> CLD_STOPPED);
white-space cleanup?
> @@ -2363,7 +2419,8 @@ bool get_signal(struct ksignal *ksig)
> * we should notify the parent, prepare_signal(SIGCONT) encodes
> * the CLD_ si_code into SIGNAL_CLD_MASK bits.
> */
> - if (unlikely(signal->flags & SIGNAL_CLD_MASK)) {
> + if (unlikely(signal->flags & SIGNAL_CLD_MASK) &
> + !(current->jobctl & JOBCTL_TRAP_FREEZE)) {
Hmm, why?
Oleg.
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v8 4/7] cgroup: cgroup v2 freezer
2019-02-20 14:42 ` Oleg Nesterov
@ 2019-02-20 22:14 ` Roman Gushchin
2019-02-21 16:44 ` Oleg Nesterov
0 siblings, 1 reply; 21+ messages in thread
From: Roman Gushchin @ 2019-02-20 22:14 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Roman Gushchin, Tejun Heo, Kernel Team, cgroups, linux-kernel
On Wed, Feb 20, 2019 at 03:42:02PM +0100, Oleg Nesterov wrote:
> On 02/19, Roman Gushchin wrote:
> >
> > @@ -2065,6 +2066,7 @@ static void ptrace_stop(int exit_code, int why, int clear_code, kernel_siginfo_t
> > preempt_disable();
> > read_unlock(&tasklist_lock);
> > preempt_enable_no_resched();
> > + cgroup_enter_stopped();
> > freezable_schedule();
> > } else {
> > /*
> > @@ -2243,6 +2245,7 @@ static bool do_signal_stop(int signr)
> > }
> >
> > /* Now we don't run again until woken by SIGCONT or SIGKILL */
> > + cgroup_enter_stopped();
> > freezable_schedule();
> > return true;
>
> Please see my reply to 0/7.
Answered in that thread.
>
> > @@ -2280,7 +2286,8 @@ static void do_jobctl_trap(void)
> > !(signal->flags & SIGNAL_STOP_STOPPED))
> > signr = SIGTRAP;
> > WARN_ON_ONCE(!signr);
> > - ptrace_do_notify(signr, signr | (PTRACE_EVENT_STOP << 8),
> > + ptrace_do_notify(signr,
> > + signr | (PTRACE_EVENT_STOP << 8),
> > CLD_STOPPED);
>
> white-space cleanup?
Sorry, a bogus change. Please, ignore it.
>
> > @@ -2363,7 +2419,8 @@ bool get_signal(struct ksignal *ksig)
> > * we should notify the parent, prepare_signal(SIGCONT) encodes
> > * the CLD_ si_code into SIGNAL_CLD_MASK bits.
> > */
> > - if (unlikely(signal->flags & SIGNAL_CLD_MASK)) {
> > + if (unlikely(signal->flags & SIGNAL_CLD_MASK) &
> > + !(current->jobctl & JOBCTL_TRAP_FREEZE)) {
>
> Hmm, why?
If a process is going from the stopped to the frozen state, it looks like
it's better to delay parent's notification up to the moment when it will
actually run.
Thanks!
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v8 4/7] cgroup: cgroup v2 freezer
2019-02-20 22:14 ` Roman Gushchin
@ 2019-02-21 16:44 ` Oleg Nesterov
0 siblings, 0 replies; 21+ messages in thread
From: Oleg Nesterov @ 2019-02-21 16:44 UTC (permalink / raw)
To: Roman Gushchin
Cc: Roman Gushchin, Tejun Heo, Kernel Team, cgroups, linux-kernel
On 02/20, Roman Gushchin wrote:
>
> On Wed, Feb 20, 2019 at 03:42:02PM +0100, Oleg Nesterov wrote:
> > On 02/19, Roman Gushchin wrote:
> > >
> > > @@ -2363,7 +2419,8 @@ bool get_signal(struct ksignal *ksig)
> > > * we should notify the parent, prepare_signal(SIGCONT) encodes
> > > * the CLD_ si_code into SIGNAL_CLD_MASK bits.
> > > */
> > > - if (unlikely(signal->flags & SIGNAL_CLD_MASK)) {
> > > + if (unlikely(signal->flags & SIGNAL_CLD_MASK) &
> > > + !(current->jobctl & JOBCTL_TRAP_FREEZE)) {
> >
> > Hmm, why?
>
> If a process is going from the stopped to the frozen state, it looks like
> it's better to delay parent's notification up to the moment when it will
> actually run.
Perhaps I missed something but I do not understand why it is really better,
at least why it deserves this (small but still) complication...
OK, if nothing else, suppose that SIGCONT races with cgroup_do_freeze(true).
With or without this change CLD_CONTINUED can be reported or not depending
on /dev/random, do we really care?
Oleg.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v8 5/7] kselftests: cgroup: don't fail on cg_kill_all() error in cg_destroy()
2019-02-19 22:02 [PATCH v8 0/7] freezer for cgroup v2 Roman Gushchin
` (3 preceding siblings ...)
2019-02-19 22:02 ` [PATCH v8 4/7] cgroup: cgroup v2 freezer Roman Gushchin
@ 2019-02-19 22:02 ` Roman Gushchin
2019-02-19 22:02 ` [PATCH v8 6/7] kselftests: cgroup: add freezer controller self-tests Roman Gushchin
` (2 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Roman Gushchin @ 2019-02-19 22:02 UTC (permalink / raw)
To: Tejun Heo, Oleg Nesterov
Cc: kernel-team, cgroups, linux-kernel, Roman Gushchin, Shuah Khan,
linux-kselftest
If the cgroup destruction races with an exit() of a belonging
process(es), cg_kill_all() may fail. It's not a good reason to make
cg_destroy() fail and leave the cgroup in place, potentially causing
next test runs to fail.
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: kernel-team@fb.com
Cc: linux-kselftest@vger.kernel.org
---
tools/testing/selftests/cgroup/cgroup_util.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/tools/testing/selftests/cgroup/cgroup_util.c b/tools/testing/selftests/cgroup/cgroup_util.c
index 14c9fe284806..eba06f94433b 100644
--- a/tools/testing/selftests/cgroup/cgroup_util.c
+++ b/tools/testing/selftests/cgroup/cgroup_util.c
@@ -227,9 +227,7 @@ int cg_destroy(const char *cgroup)
retry:
ret = rmdir(cgroup);
if (ret && errno == EBUSY) {
- ret = cg_killall(cgroup);
- if (ret)
- return ret;
+ cg_killall(cgroup);
usleep(100);
goto retry;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v8 6/7] kselftests: cgroup: add freezer controller self-tests
2019-02-19 22:02 [PATCH v8 0/7] freezer for cgroup v2 Roman Gushchin
` (4 preceding siblings ...)
2019-02-19 22:02 ` [PATCH v8 5/7] kselftests: cgroup: don't fail on cg_kill_all() error in cg_destroy() Roman Gushchin
@ 2019-02-19 22:02 ` Roman Gushchin
2019-02-19 22:02 ` [PATCH v8 7/7] cgroup: document cgroup v2 freezer interface Roman Gushchin
2019-02-20 14:37 ` [PATCH v8 0/7] freezer for cgroup v2 Oleg Nesterov
7 siblings, 0 replies; 21+ messages in thread
From: Roman Gushchin @ 2019-02-19 22:02 UTC (permalink / raw)
To: Tejun Heo, Oleg Nesterov
Cc: kernel-team, cgroups, linux-kernel, Roman Gushchin, Shuah Khan,
linux-kselftest
This patch implements 8 tests for the freezer controller for
cgroup v2:
1) a simple test, which aims to freeze and unfreeze a cgroup with 100
processes
2) a more complicated tree test, which creates a hierarchy of cgroups,
puts some processes in some cgroups, and tries to freeze and unfreeze
different parts of the subtree
3) a forkbomb test: the test aims to freeze a forkbomb running in a
cgroup, kill all tasks in the cgroup and remove the cgroup without
the unfreezing.
4) rmdir test: the test creates two nested cgroups, freezes the parent
one, checks that the child can be successfully removed, and a new
child can be created
5) migration tests: the test checks migration of a task between
frozen cgroups: from a frozen to a running, from a running to a
frozen, and from a frozen to a frozen.
6) ptrace test: the test checks that it's possible to attach to
a process in a frozen cgroup, get some information and detach, and
the cgroup will remain frozen.
7) stopped test: the test checks that it's possible to freeze a cgroup
with a stopped task
8) ptraced test: the test checks that it's possible to freeze a cgroup
with a ptraced task
Expected output:
$ ./test_freezer
ok 1 test_cgfreezer_simple
ok 2 test_cgfreezer_tree
ok 3 test_cgfreezer_forkbomb
ok 4 test_cgrreezer_rmdir
ok 5 test_cgfreezer_migrate
ok 6 test_cgfreezer_ptrace
ok 7 test_cgfreezer_stopped
ok 8 test_cgfreezer_ptraced
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: kernel-team@fb.com
Cc: linux-kselftest@vger.kernel.org
---
tools/testing/selftests/cgroup/.gitignore | 1 +
tools/testing/selftests/cgroup/Makefile | 2 +
tools/testing/selftests/cgroup/cgroup_util.c | 81 +-
tools/testing/selftests/cgroup/cgroup_util.h | 7 +
tools/testing/selftests/cgroup/test_freezer.c | 842 ++++++++++++++++++
5 files changed, 932 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/cgroup/test_freezer.c
diff --git a/tools/testing/selftests/cgroup/.gitignore b/tools/testing/selftests/cgroup/.gitignore
index adacda50a4b2..7f9835624793 100644
--- a/tools/testing/selftests/cgroup/.gitignore
+++ b/tools/testing/selftests/cgroup/.gitignore
@@ -1,2 +1,3 @@
test_memcontrol
test_core
+test_freezer
diff --git a/tools/testing/selftests/cgroup/Makefile b/tools/testing/selftests/cgroup/Makefile
index 23fbaa4a9630..8d369b6a2069 100644
--- a/tools/testing/selftests/cgroup/Makefile
+++ b/tools/testing/selftests/cgroup/Makefile
@@ -5,8 +5,10 @@ all:
TEST_GEN_PROGS = test_memcontrol
TEST_GEN_PROGS += test_core
+TEST_GEN_PROGS += test_freezer
include ../lib.mk
$(OUTPUT)/test_memcontrol: cgroup_util.c
$(OUTPUT)/test_core: cgroup_util.c
+$(OUTPUT)/test_freezer: cgroup_util.c
diff --git a/tools/testing/selftests/cgroup/cgroup_util.c b/tools/testing/selftests/cgroup/cgroup_util.c
index eba06f94433b..e9cdad673901 100644
--- a/tools/testing/selftests/cgroup/cgroup_util.c
+++ b/tools/testing/selftests/cgroup/cgroup_util.c
@@ -74,6 +74,16 @@ char *cg_name_indexed(const char *root, const char *name, int index)
return ret;
}
+char *cg_control(const char *cgroup, const char *control)
+{
+ size_t len = strlen(cgroup) + strlen(control) + 2;
+ char *ret = malloc(len);
+
+ snprintf(ret, len, "%s/%s", cgroup, control);
+
+ return ret;
+}
+
int cg_read(const char *cgroup, const char *control, char *buf, size_t len)
{
char path[PATH_MAX];
@@ -196,7 +206,59 @@ int cg_create(const char *cgroup)
return mkdir(cgroup, 0644);
}
-static int cg_killall(const char *cgroup)
+int cg_for_all_procs(const char *cgroup, int (*fn)(int pid, void *arg),
+ void *arg)
+{
+ char buf[PAGE_SIZE];
+ char *ptr = buf;
+ int ret;
+
+ if (cg_read(cgroup, "cgroup.procs", buf, sizeof(buf)))
+ return -1;
+
+ while (ptr < buf + sizeof(buf)) {
+ int pid = strtol(ptr, &ptr, 10);
+
+ if (pid == 0)
+ break;
+ if (*ptr)
+ ptr++;
+ else
+ break;
+ ret = fn(pid, arg);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+int cg_wait_for_proc_count(const char *cgroup, int count)
+{
+ char buf[10 * PAGE_SIZE] = {0};
+ int attempts;
+ char *ptr;
+
+ for (attempts = 10; attempts >= 0; attempts--) {
+ int nr = 0;
+
+ if (cg_read(cgroup, "cgroup.procs", buf, sizeof(buf)))
+ break;
+
+ for (ptr = buf; *ptr; ptr++)
+ if (*ptr == '\n')
+ nr++;
+
+ if (nr >= count)
+ return 0;
+
+ usleep(100000);
+ }
+
+ return -1;
+}
+
+int cg_killall(const char *cgroup)
{
char buf[PAGE_SIZE];
char *ptr = buf;
@@ -238,6 +300,14 @@ int cg_destroy(const char *cgroup)
return ret;
}
+int cg_enter(const char *cgroup, int pid)
+{
+ char pidbuf[64];
+
+ snprintf(pidbuf, sizeof(pidbuf), "%d", pid);
+ return cg_write(cgroup, "cgroup.procs", pidbuf);
+}
+
int cg_enter_current(const char *cgroup)
{
char pidbuf[64];
@@ -367,3 +437,12 @@ int set_oom_adj_score(int pid, int score)
close(fd);
return 0;
}
+
+char proc_read_text(int pid, const char *item, char *buf, size_t size)
+{
+ char path[PATH_MAX];
+
+ snprintf(path, sizeof(path), "/proc/%d/%s", pid, item);
+
+ return read_text(path, buf, size);
+}
diff --git a/tools/testing/selftests/cgroup/cgroup_util.h b/tools/testing/selftests/cgroup/cgroup_util.h
index 9ac8b7958f83..8ee63c00a668 100644
--- a/tools/testing/selftests/cgroup/cgroup_util.h
+++ b/tools/testing/selftests/cgroup/cgroup_util.h
@@ -18,6 +18,7 @@ static inline int values_close(long a, long b, int err)
extern int cg_find_unified_root(char *root, size_t len);
extern char *cg_name(const char *root, const char *name);
extern char *cg_name_indexed(const char *root, const char *name, int index);
+extern char *cg_control(const char *cgroup, const char *control);
extern int cg_create(const char *cgroup);
extern int cg_destroy(const char *cgroup);
extern int cg_read(const char *cgroup, const char *control,
@@ -32,6 +33,7 @@ extern int cg_write(const char *cgroup, const char *control, char *buf);
extern int cg_run(const char *cgroup,
int (*fn)(const char *cgroup, void *arg),
void *arg);
+extern int cg_enter(const char *cgroup, int pid);
extern int cg_enter_current(const char *cgroup);
extern int cg_run_nowait(const char *cgroup,
int (*fn)(const char *cgroup, void *arg),
@@ -41,3 +43,8 @@ extern int alloc_pagecache(int fd, size_t size);
extern int alloc_anon(const char *cgroup, void *arg);
extern int is_swap_enabled(void);
extern int set_oom_adj_score(int pid, int score);
+extern int cg_for_all_procs(const char *cgroup, int (*fn)(int pid, void *arg),
+ void *arg);
+extern int cg_wait_for_proc_count(const char *cgroup, int count);
+extern int cg_killall(const char *cgroup);
+extern char proc_read_text(int pid, const char *item, char *buf, size_t size);
diff --git a/tools/testing/selftests/cgroup/test_freezer.c b/tools/testing/selftests/cgroup/test_freezer.c
new file mode 100644
index 000000000000..c11176a92474
--- /dev/null
+++ b/tools/testing/selftests/cgroup/test_freezer.c
@@ -0,0 +1,842 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#include <stdbool.h>
+#include <linux/limits.h>
+#include <sys/ptrace.h>
+#include <sys/types.h>
+#include <sys/mman.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <errno.h>
+#include <poll.h>
+#include <stdlib.h>
+#include <sys/inotify.h>
+#include <string.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+#include "../kselftest.h"
+#include "cgroup_util.h"
+
+#define DEBUG
+#ifdef DEBUG
+#define debug(args...) fprintf(stderr, args)
+#else
+#define debug(args...)
+#endif
+
+/*
+ * Freeze the given cgroup and wait for the inotify signal.
+ * If there is no signal in 10 seconds, treat this as an error.
+ */
+static int cg_freeze_wait(const char *cgroup, bool freeze)
+{
+ int fd, wd;
+ struct pollfd fds;
+ int ret = -1;
+
+ fd = inotify_init1(IN_NONBLOCK);
+ if (fd == -1)
+ return fd;
+
+ wd = inotify_add_watch(fd, cg_control(cgroup, "cgroup.events"),
+ IN_MODIFY);
+ if (wd == -1) {
+ close(fd);
+ return wd;
+ }
+ fds.fd = fd;
+ fds.events = POLLIN;
+
+ ret = cg_write(cgroup, "cgroup.freeze", freeze ? "1" : "0");
+ if (ret) {
+ close(fd);
+ return ret;
+ }
+
+ while (true) {
+ wd = poll(&fds, 1, 10000);
+
+ if (wd == -1 && errno == EINTR)
+ continue;
+
+ if (wd == 1 && fds.revents & POLLIN)
+ ret = 0;
+
+ break;
+ }
+
+ close(fd);
+
+ return ret;
+}
+
+/*
+ * Check if the process is frozen and parked in a proper place.
+ */
+static int proc_check_frozen(int pid, void *arg)
+{
+ char buf[PAGE_SIZE];
+ int len;
+
+ len = proc_read_text(pid, "stat", buf, sizeof(buf));
+ if (len == -1) {
+ debug("Can't get %d stat\n", pid);
+ return -1;
+ }
+
+ if (strstr(buf, "(test_freezer) S ") == NULL) {
+ debug("Process %d in the unexpected state: %s\n", pid, buf);
+ return -1;
+ }
+
+ len = proc_read_text(pid, "stack", buf, sizeof(buf));
+ if (len == -1) {
+ debug("Can't get stack of the process %d\n", pid);
+ return -1;
+ }
+
+ if (strstr(buf, "[<0>] get_signal") != buf) {
+ debug("Process %d has unexpected stacktrace: %s\n", pid, buf);
+ return -1;
+ }
+
+ return 0;
+}
+
+/*
+ * Check if the cgroup is frozen by looking at the cgroup.events::frozen value.
+ */
+static int __cg_check_frozen(const char *cgroup, bool frozen)
+{
+ if (frozen) {
+ if (cg_read_strstr(cgroup, "cgroup.events", "frozen 1") != 0) {
+ debug("Cgroup %s isn't unexpectedly frozen\n", cgroup);
+ return -1;
+ }
+ } else {
+ /*
+ * Check the cgroup.events::frozen value.
+ */
+ if (cg_read_strstr(cgroup, "cgroup.events", "frozen 0") != 0) {
+ debug("Cgroup %s is unexpectedly frozen\n", cgroup);
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
+/*
+ * Check if the cgroup is frozen and all belonging processes are
+ * parked in a proper place.
+ */
+static int cg_check_frozen(const char *cgroup, bool frozen)
+{
+ if (__cg_check_frozen(cgroup, frozen))
+ return -1;
+
+ /*
+ * Check that all processes are parked in the proper place.
+ */
+ if (frozen && cg_for_all_procs(cgroup, proc_check_frozen, NULL)) {
+ debug("Some processes of cgroup %s are not frozen\n",
+ cgroup);
+ return -1;
+ }
+
+ return 0;
+}
+
+/*
+ * A simple process running in a sleep loop until being
+ * re-parented.
+ */
+static int child_fn(const char *cgroup, void *arg)
+{
+ int ppid = getppid();
+
+ while (getppid() == ppid)
+ usleep(1000);
+
+ return getppid() == ppid;
+}
+
+/*
+ * A simple test for the cgroup freezer: populated the cgroup with 100
+ * running processes and freeze it. Then unfreeze it. Then it kills all
+ * processes and destroys the cgroup.
+ */
+static int test_cgfreezer_simple(const char *root)
+{
+ int ret = KSFT_FAIL;
+ char *cgroup = NULL;
+ int i;
+
+ cgroup = cg_name(root, "cg_test");
+ if (!cgroup)
+ goto cleanup;
+
+ if (cg_create(cgroup))
+ goto cleanup;
+
+ for (i = 0; i < 100; i++)
+ cg_run_nowait(cgroup, child_fn, NULL);
+
+ if (cg_wait_for_proc_count(cgroup, 100))
+ goto cleanup;
+
+ if (cg_check_frozen(cgroup, false))
+ goto cleanup;
+
+ if (cg_freeze_wait(cgroup, true))
+ goto cleanup;
+
+ if (cg_check_frozen(cgroup, true))
+ goto cleanup;
+
+ if (cg_freeze_wait(cgroup, false))
+ goto cleanup;
+
+ if (cg_check_frozen(cgroup, false))
+ goto cleanup;
+
+ ret = KSFT_PASS;
+
+cleanup:
+ if (cgroup)
+ cg_destroy(cgroup);
+ free(cgroup);
+ return ret;
+}
+
+/*
+ * The test creates the following hierarchy:
+ * A
+ * / / \ \
+ * B E I K
+ * /\ |
+ * C D F
+ * |
+ * G
+ * |
+ * H
+ *
+ * with a process in C, H and 3 processes in K.
+ * Then it tries to freeze and unfreeze the whole tree.
+ */
+static int test_cgfreezer_tree(const char *root)
+{
+ char *cgroup[10] = {0};
+ int ret = KSFT_FAIL;
+ int i;
+
+ cgroup[0] = cg_name(root, "cg_test_A");
+ if (!cgroup[0])
+ goto cleanup;
+
+ cgroup[1] = cg_name(cgroup[0], "cg_test_B");
+ if (!cgroup[1])
+ goto cleanup;
+
+ cgroup[2] = cg_name(cgroup[1], "cg_test_C");
+ if (!cgroup[2])
+ goto cleanup;
+
+ cgroup[3] = cg_name(cgroup[1], "cg_test_D");
+ if (!cgroup[3])
+ goto cleanup;
+
+ cgroup[4] = cg_name(cgroup[0], "cg_test_E");
+ if (!cgroup[4])
+ goto cleanup;
+
+ cgroup[5] = cg_name(cgroup[4], "cg_test_F");
+ if (!cgroup[5])
+ goto cleanup;
+
+ cgroup[6] = cg_name(cgroup[5], "cg_test_G");
+ if (!cgroup[6])
+ goto cleanup;
+
+ cgroup[7] = cg_name(cgroup[6], "cg_test_H");
+ if (!cgroup[7])
+ goto cleanup;
+
+ cgroup[8] = cg_name(cgroup[0], "cg_test_I");
+ if (!cgroup[8])
+ goto cleanup;
+
+ cgroup[9] = cg_name(cgroup[0], "cg_test_K");
+ if (!cgroup[9])
+ goto cleanup;
+
+ for (i = 0; i < 10; i++)
+ if (cg_create(cgroup[i]))
+ goto cleanup;
+
+ cg_run_nowait(cgroup[2], child_fn, NULL);
+ cg_run_nowait(cgroup[7], child_fn, NULL);
+ cg_run_nowait(cgroup[9], child_fn, NULL);
+ cg_run_nowait(cgroup[9], child_fn, NULL);
+ cg_run_nowait(cgroup[9], child_fn, NULL);
+
+ /*
+ * Wait until all child processes will enter
+ * corresponding cgroups.
+ */
+
+ if (cg_wait_for_proc_count(cgroup[2], 1) ||
+ cg_wait_for_proc_count(cgroup[7], 1) ||
+ cg_wait_for_proc_count(cgroup[9], 3))
+ goto cleanup;
+
+ /*
+ * Freeze B.
+ */
+ if (cg_freeze_wait(cgroup[1], true))
+ goto cleanup;
+
+ if (cg_check_frozen(cgroup[1], true))
+ goto cleanup;
+
+ /*
+ * Freeze F.
+ */
+ if (cg_freeze_wait(cgroup[5], true))
+ goto cleanup;
+
+ if (cg_check_frozen(cgroup[5], true))
+ goto cleanup;
+
+ /*
+ * Freeze G.
+ */
+ if (cg_freeze_wait(cgroup[6], true))
+ goto cleanup;
+
+ if (cg_check_frozen(cgroup[6], true))
+ goto cleanup;
+
+ /*
+ * Check that A and E are not frozen.
+ */
+ if (cg_check_frozen(cgroup[0], false))
+ goto cleanup;
+
+ if (cg_check_frozen(cgroup[4], false))
+ goto cleanup;
+
+ /*
+ * Freeze A. Check that A, B and E are frozen.
+ */
+ if (cg_freeze_wait(cgroup[0], true))
+ goto cleanup;
+
+ if (cg_check_frozen(cgroup[0], true))
+ goto cleanup;
+
+ if (cg_check_frozen(cgroup[1], true))
+ goto cleanup;
+
+ if (cg_check_frozen(cgroup[4], true))
+ goto cleanup;
+
+ /*
+ * Unfreeze B, F and G
+ */
+ if (cg_freeze_wait(cgroup[1], false))
+ goto cleanup;
+
+ if (cg_freeze_wait(cgroup[5], false))
+ goto cleanup;
+
+ if (cg_freeze_wait(cgroup[6], false))
+ goto cleanup;
+
+ /*
+ * Check that C and H are still frozen.
+ */
+ if (cg_check_frozen(cgroup[2], true))
+ goto cleanup;
+
+ if (cg_check_frozen(cgroup[7], true))
+ goto cleanup;
+
+ /*
+ * Unfreezing A failed. Check that A, C and K are not frozen.
+ */
+ if (cg_freeze_wait(cgroup[0], false))
+ goto cleanup;
+
+ if (cg_check_frozen(cgroup[0], false))
+ goto cleanup;
+
+ if (cg_check_frozen(cgroup[2], false))
+ goto cleanup;
+
+ if (cg_check_frozen(cgroup[9], false))
+ goto cleanup;
+
+ ret = KSFT_PASS;
+
+cleanup:
+ for (i = 9; i >= 0 && cgroup[i]; i--) {
+ cg_destroy(cgroup[i]);
+ free(cgroup[i]);
+ }
+
+ return ret;
+}
+
+/*
+ * A fork bomb emulator.
+ */
+static int forkbomb_fn(const char *cgroup, void *arg)
+{
+ int ppid;
+
+ fork();
+ fork();
+
+ ppid = getppid();
+
+ while (getppid() == ppid)
+ usleep(1000);
+
+ return getppid() == ppid;
+}
+
+/*
+ * The test runs a fork bomb in a cgroup and tries to freeze it.
+ * Then it kills all processes and checks that cgroup isn't populated
+ * anymore.
+ */
+static int test_cgfreezer_forkbomb(const char *root)
+{
+ int ret = KSFT_FAIL;
+ char *cgroup = NULL;
+
+ cgroup = cg_name(root, "cg_forkbomb_test");
+ if (!cgroup)
+ goto cleanup;
+
+ if (cg_create(cgroup))
+ goto cleanup;
+
+ cg_run_nowait(cgroup, forkbomb_fn, NULL);
+
+ usleep(100000);
+
+ if (cg_freeze_wait(cgroup, true))
+ goto cleanup;
+
+ if (cg_check_frozen(cgroup, true))
+ goto cleanup;
+
+ if (cg_killall(cgroup))
+ goto cleanup;
+
+ if (cg_wait_for_proc_count(cgroup, 0))
+ goto cleanup;
+
+ ret = KSFT_PASS;
+
+cleanup:
+ if (cgroup)
+ cg_destroy(cgroup);
+ free(cgroup);
+ return ret;
+}
+
+/*
+ * The test creates two nested cgroups, freezes the parent
+ * and removes the child. Then it checks that the parent cgroup
+ * remains frozen and it's possible to create a new child
+ * without unfreezing. The new child is frozen too.
+ */
+static int test_cgfreezer_rmdir(const char *root)
+{
+ int ret = KSFT_FAIL;
+ char *parent, *child = NULL;
+
+ parent = cg_name(root, "cg_test_A");
+ if (!parent)
+ goto cleanup;
+
+ child = cg_name(parent, "cg_test_B");
+ if (!child)
+ goto cleanup;
+
+ if (cg_create(parent))
+ goto cleanup;
+
+ if (cg_create(child))
+ goto cleanup;
+
+ if (cg_freeze_wait(parent, true))
+ goto cleanup;
+
+ if (cg_check_frozen(parent, true))
+ goto cleanup;
+
+ if (cg_destroy(child))
+ goto cleanup;
+
+ if (cg_check_frozen(parent, true))
+ goto cleanup;
+
+ if (cg_create(child))
+ goto cleanup;
+
+ if (cg_check_frozen(child, true))
+ goto cleanup;
+
+ ret = KSFT_PASS;
+
+cleanup:
+ if (child)
+ cg_destroy(child);
+ free(child);
+ if (parent)
+ cg_destroy(parent);
+ free(parent);
+ return ret;
+}
+
+/*
+ * The test creates two cgroups: A and B. The it runs a process in A,
+ * and performs several migrations:
+ * 1) A (running) -> B (frozen)
+ * 2) B (frozen) -> A (running)
+ * 3) A (frozen) -> B (frozen)
+ *
+ * One each step it checks that the actual state of cgroups matches
+ * the expected state.
+ */
+static int test_cgfreezer_migrate(const char *root)
+{
+ int ret = KSFT_FAIL;
+ char *cgroup[2] = {0};
+ int pid;
+
+ cgroup[0] = cg_name(root, "cg_test_A");
+ if (!cgroup[0])
+ goto cleanup;
+
+ cgroup[1] = cg_name(root, "cg_test_B");
+ if (!cgroup[1])
+ goto cleanup;
+
+ if (cg_create(cgroup[0]))
+ goto cleanup;
+
+ if (cg_create(cgroup[1]))
+ goto cleanup;
+
+ pid = cg_run_nowait(cgroup[0], child_fn, NULL);
+ if (pid < 0)
+ goto cleanup;
+
+ if (cg_wait_for_proc_count(cgroup[0], 1))
+ goto cleanup;
+
+ /*
+ * Migrate from A (running) to B (frozen)
+ */
+ if (cg_freeze_wait(cgroup[1], true))
+ goto cleanup;
+
+ if (cg_check_frozen(cgroup[1], true))
+ goto cleanup;
+
+ if (cg_enter(cgroup[1], pid))
+ goto cleanup;
+
+ if (cg_check_frozen(cgroup[0], false))
+ goto cleanup;
+
+ if (cg_check_frozen(cgroup[1], true))
+ goto cleanup;
+
+ /*
+ * Migrate from B (frozen) to A (running)
+ */
+ if (cg_enter(cgroup[0], pid))
+ goto cleanup;
+
+ if (cg_check_frozen(cgroup[0], false))
+ goto cleanup;
+
+ if (cg_check_frozen(cgroup[1], true))
+ goto cleanup;
+
+ /*
+ * Migrate from A (frozen) to B (frozen)
+ */
+ if (cg_freeze_wait(cgroup[0], true))
+ goto cleanup;
+
+ if (cg_check_frozen(cgroup[0], true))
+ goto cleanup;
+
+ if (cg_enter(cgroup[1], pid))
+ goto cleanup;
+
+ if (cg_check_frozen(cgroup[0], true))
+ goto cleanup;
+
+ if (cg_check_frozen(cgroup[1], true))
+ goto cleanup;
+
+ ret = KSFT_PASS;
+
+cleanup:
+ if (cgroup[0])
+ cg_destroy(cgroup[0]);
+ free(cgroup[0]);
+ if (cgroup[1])
+ cg_destroy(cgroup[1]);
+ free(cgroup[1]);
+ return ret;
+}
+
+/*
+ * The test checks that ptrace works with a tracing process in a frozen cgroup.
+ */
+static int test_cgfreezer_ptrace(const char *root)
+{
+ int ret = KSFT_FAIL;
+ char *cgroup = NULL;
+ siginfo_t siginfo;
+ int pid;
+
+ cgroup = cg_name(root, "cg_test");
+ if (!cgroup)
+ goto cleanup;
+
+ if (cg_create(cgroup))
+ goto cleanup;
+
+ pid = cg_run_nowait(cgroup, child_fn, NULL);
+ if (pid < 0)
+ goto cleanup;
+
+ if (cg_wait_for_proc_count(cgroup, 1))
+ goto cleanup;
+
+ if (cg_freeze_wait(cgroup, true))
+ goto cleanup;
+
+ if (cg_check_frozen(cgroup, true))
+ goto cleanup;
+
+ if (ptrace(PTRACE_SEIZE, pid, NULL, NULL))
+ goto cleanup;
+
+ if (ptrace(PTRACE_INTERRUPT, pid, NULL, NULL))
+ goto cleanup;
+
+ waitpid(pid, NULL, 0);
+
+ /*
+ * Cgroup has to remain frozen, however the test task
+ * is in traced state.
+ */
+ if (__cg_check_frozen(cgroup, true))
+ goto cleanup;
+
+ if (ptrace(PTRACE_GETSIGINFO, pid, NULL, &siginfo))
+ goto cleanup;
+
+ if (ptrace(PTRACE_DETACH, pid, NULL, NULL))
+ goto cleanup;
+
+ if (cg_check_frozen(cgroup, true))
+ goto cleanup;
+
+ ret = KSFT_PASS;
+
+cleanup:
+ if (cgroup)
+ cg_destroy(cgroup);
+ free(cgroup);
+ return ret;
+}
+
+/*
+ * Check if the process is stopped.
+ */
+static int proc_check_stopped(int pid)
+{
+ char buf[PAGE_SIZE];
+ int len;
+
+ len = proc_read_text(pid, "stat", buf, sizeof(buf));
+ if (len == -1) {
+ debug("Can't get %d stat\n", pid);
+ return -1;
+ }
+
+ if (strstr(buf, "(test_freezer) T ") == NULL) {
+ debug("Process %d in the unexpected state: %s\n", pid, buf);
+ return -1;
+ }
+
+ return 0;
+}
+
+/*
+ * Test that it's possible to freeze a cgroup with a stopped process.
+ */
+static int test_cgfreezer_stopped(const char *root)
+{
+ int pid, ret = KSFT_FAIL;
+ char *cgroup = NULL;
+
+ cgroup = cg_name(root, "cg_test");
+ if (!cgroup)
+ goto cleanup;
+
+ if (cg_create(cgroup))
+ goto cleanup;
+
+ pid = cg_run_nowait(cgroup, child_fn, NULL);
+
+ if (cg_wait_for_proc_count(cgroup, 1))
+ goto cleanup;
+
+ if (kill(pid, SIGSTOP))
+ goto cleanup;
+
+ if (cg_check_frozen(cgroup, false))
+ goto cleanup;
+
+ if (cg_freeze_wait(cgroup, true))
+ goto cleanup;
+
+ /*
+ * cg_check_frozen(cgroup, true) will fail here,
+ * because the task in in the STOPped state;
+ * but the cgroup is expected to be frozen.
+ */
+ if (__cg_check_frozen(cgroup, true))
+ goto cleanup;
+
+ if (cg_freeze_wait(cgroup, false))
+ goto cleanup;
+
+ if (cg_check_frozen(cgroup, false))
+ goto cleanup;
+
+ if (proc_check_stopped(pid))
+ goto cleanup;
+
+ ret = KSFT_PASS;
+
+cleanup:
+ if (cgroup)
+ cg_destroy(cgroup);
+ free(cgroup);
+ return ret;
+}
+
+/*
+ * Test that it's possible to freeze a cgroup with a ptraced process.
+ */
+static int test_cgfreezer_ptraced(const char *root)
+{
+ int pid, ret = KSFT_FAIL;
+ char *cgroup = NULL;
+ siginfo_t siginfo;
+
+ cgroup = cg_name(root, "cg_test");
+ if (!cgroup)
+ goto cleanup;
+
+ if (cg_create(cgroup))
+ goto cleanup;
+
+ pid = cg_run_nowait(cgroup, child_fn, NULL);
+
+ if (cg_wait_for_proc_count(cgroup, 1))
+ goto cleanup;
+
+ if (ptrace(PTRACE_SEIZE, pid, NULL, NULL))
+ goto cleanup;
+
+ if (ptrace(PTRACE_INTERRUPT, pid, NULL, NULL))
+ goto cleanup;
+
+ waitpid(pid, NULL, 0);
+
+ if (cg_check_frozen(cgroup, false))
+ goto cleanup;
+
+ if (cg_freeze_wait(cgroup, true))
+ goto cleanup;
+
+ /*
+ * cg_check_frozen(cgroup, true) will fail here,
+ * because the task in in the TRACEd state.
+ */
+ if (cg_freeze_wait(cgroup, false))
+ goto cleanup;
+
+ if (cg_check_frozen(cgroup, false))
+ goto cleanup;
+
+ if (ptrace(PTRACE_GETSIGINFO, pid, NULL, &siginfo))
+ goto cleanup;
+
+ if (ptrace(PTRACE_DETACH, pid, NULL, NULL))
+ goto cleanup;
+
+ ret = KSFT_PASS;
+
+cleanup:
+ if (cgroup)
+ cg_destroy(cgroup);
+ free(cgroup);
+ return ret;
+}
+
+#define T(x) { x, #x }
+struct cgfreezer_test {
+ int (*fn)(const char *root);
+ const char *name;
+} tests[] = {
+ T(test_cgfreezer_simple),
+ T(test_cgfreezer_tree),
+ T(test_cgfreezer_forkbomb),
+ T(test_cgfreezer_rmdir),
+ T(test_cgfreezer_migrate),
+ T(test_cgfreezer_ptrace),
+ T(test_cgfreezer_stopped),
+ T(test_cgfreezer_ptraced),
+};
+#undef T
+
+int main(int argc, char *argv[])
+{
+ char root[PATH_MAX];
+ int i, ret = EXIT_SUCCESS;
+
+ if (cg_find_unified_root(root, sizeof(root)))
+ ksft_exit_skip("cgroup v2 isn't mounted\n");
+ for (i = 0; i < ARRAY_SIZE(tests); i++) {
+ switch (tests[i].fn(root)) {
+ case KSFT_PASS:
+ ksft_test_result_pass("%s\n", tests[i].name);
+ break;
+ case KSFT_SKIP:
+ ksft_test_result_skip("%s\n", tests[i].name);
+ break;
+ default:
+ ret = EXIT_FAILURE;
+ ksft_test_result_fail("%s\n", tests[i].name);
+ break;
+ }
+ }
+
+ return ret;
+}
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v8 7/7] cgroup: document cgroup v2 freezer interface
2019-02-19 22:02 [PATCH v8 0/7] freezer for cgroup v2 Roman Gushchin
` (5 preceding siblings ...)
2019-02-19 22:02 ` [PATCH v8 6/7] kselftests: cgroup: add freezer controller self-tests Roman Gushchin
@ 2019-02-19 22:02 ` Roman Gushchin
2019-02-20 14:37 ` [PATCH v8 0/7] freezer for cgroup v2 Oleg Nesterov
7 siblings, 0 replies; 21+ messages in thread
From: Roman Gushchin @ 2019-02-19 22:02 UTC (permalink / raw)
To: Tejun Heo, Oleg Nesterov
Cc: kernel-team, cgroups, linux-kernel, Roman Gushchin,
Mike Rapoport, linux-doc
Describe cgroup v2 freezer interface in the cgroup v2 admin guide.
Signed-off-by: Roman Gushchin <guro@fb.com>
Reviewed-by: Mike Rapoport <rppt@linux.ibm.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: linux-doc@vger.kernel.org
Cc: kernel-team@fb.com
---
Documentation/admin-guide/cgroup-v2.rst | 27 +++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
index 61f8bbb0a1b2..78f078ddbe9c 100644
--- a/Documentation/admin-guide/cgroup-v2.rst
+++ b/Documentation/admin-guide/cgroup-v2.rst
@@ -864,6 +864,8 @@ All cgroup core files are prefixed with "cgroup."
populated
1 if the cgroup or its descendants contains any live
processes; otherwise, 0.
+ frozen
+ 1 if the cgroup is frozen; otherwise, 0.
cgroup.max.descendants
A read-write single value files. The default is "max".
@@ -897,6 +899,31 @@ All cgroup core files are prefixed with "cgroup."
A dying cgroup can consume system resources not exceeding
limits, which were active at the moment of cgroup deletion.
+ cgroup.freeze
+ A read-write single value file which exists on non-root cgroups.
+ Allowed values are "0" and "1". The default is "0".
+
+ Writing "1" to the file causes freezing of the cgroup and all
+ descendant cgroups. This means that all belonging processes will
+ be stopped and will not run until the cgroup will be explicitly
+ unfrozen. Freezing of the cgroup may take some time; when this action
+ is completed, the "frozen" value in the cgroup.events control file
+ will be updated to "1" and the corresponding notification will be
+ issued.
+
+ A cgroup can be frozen either by its own settings, or by settings
+ of any ancestor cgroups. If any of ancestor cgroups is frozen, the
+ cgroup will remain frozen.
+
+ Processes in the frozen cgroup can be killed by a fatal signal.
+ They also can enter and leave a frozen cgroup: either by an explicit
+ move by a user, or if freezing of the cgroup races with fork().
+ If a process is moved to a frozen cgroup, it stops. If a process is
+ moved out of a frozen cgroup, it becomes running.
+
+ Frozen status of a cgroup doesn't affect any cgroup tree operations:
+ it's possible to delete a frozen (and empty) cgroup, as well as
+ create new sub-cgroups.
Controllers
===========
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v8 0/7] freezer for cgroup v2
2019-02-19 22:02 [PATCH v8 0/7] freezer for cgroup v2 Roman Gushchin
` (6 preceding siblings ...)
2019-02-19 22:02 ` [PATCH v8 7/7] cgroup: document cgroup v2 freezer interface Roman Gushchin
@ 2019-02-20 14:37 ` Oleg Nesterov
2019-02-20 22:00 ` Roman Gushchin
7 siblings, 1 reply; 21+ messages in thread
From: Oleg Nesterov @ 2019-02-20 14:37 UTC (permalink / raw)
To: Roman Gushchin
Cc: Tejun Heo, kernel-team, cgroups, linux-kernel, Roman Gushchin
On 02/19, Roman Gushchin wrote:
>
> It provides similar functionality as v1 freezer, but the interface
> conforms to the cgroup v2 interface design principles, and it
> provides a better user experience: tasks can be killed, ptrace works,
I tried to not argue with intent, but to be honest I am more and more
sceptical... Lets forget about ptrace for the moment.
Once again, why do we want a killable freezer?
If a user wants to kill a frozen task from CGRP_FROZEN cgroup he can simply
1. send SIGKILL to that task
2. migrate it to the root cgroup.
why this doesn't / can't work?
Why I am starting to argue... The ability to kill a frozen task complicates
the code, and since cgroup_enter_stopped() (in this version at least) doesn't
properly interacts with freezable_schedule() leads to other problems.
From 7/7:
+ cgroup.freeze
+ A read-write single value file which exists on non-root cgroups.
+ Allowed values are "0" and "1". The default is "0".
+
+ Writing "1" to the file causes freezing of the cgroup and all
+ descendant cgroups. This means that all belonging processes will
+ be stopped and will not run until the cgroup will be explicitly
+ unfrozen. Freezing of the cgroup may take some time;
^^^^^^^^^^^^^^^^^^
it may take infinite time.
Just suppose that a task does vfork() and this races with cgroup_do_freeze(true).
If the new child notices JOBCTL_TRAP_FREEZE before exit/exec the cgroup will be
never frozen.
If I read the current kernel/cgroup/freezer.c correctly, CGROUP_FREEZING should
"always" work (unless a task hangs in D state) and to me this looks more important
than kill/ptrace support...
> there is no separate controller, which has to be enabled, etc.
agreed, this is nice.
Oleg.
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v8 0/7] freezer for cgroup v2
2019-02-20 14:37 ` [PATCH v8 0/7] freezer for cgroup v2 Oleg Nesterov
@ 2019-02-20 22:00 ` Roman Gushchin
2019-02-21 16:29 ` Oleg Nesterov
0 siblings, 1 reply; 21+ messages in thread
From: Roman Gushchin @ 2019-02-20 22:00 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Roman Gushchin, Tejun Heo, Kernel Team, cgroups, linux-kernel
On Wed, Feb 20, 2019 at 03:37:48PM +0100, Oleg Nesterov wrote:
> On 02/19, Roman Gushchin wrote:
> >
> > It provides similar functionality as v1 freezer, but the interface
> > conforms to the cgroup v2 interface design principles, and it
> > provides a better user experience: tasks can be killed, ptrace works,
>
> I tried to not argue with intent, but to be honest I am more and more
> sceptical... Lets forget about ptrace for the moment.
>
> Once again, why do we want a killable freezer?
>
> If a user wants to kill a frozen task from CGRP_FROZEN cgroup he can simply
>
> 1. send SIGKILL to that task
>
> 2. migrate it to the root cgroup.
>
> why this doesn't / can't work?
It does work, but it doesn't look as a nice interface to take into
the cgroup v2 world.
It just not clear, why killing a frozen task requires some cgroup-level
operations? It doesn't add anything except some additional complexity
to the userspace. Generally speaking, any process hanging in D-state
for a long time isn't the nicest object from the userspace's point of view.
Exactly as a SIGSTOPped process can be killed without sending SIGCONT,
why a frozen task would require some additional operations?
And I'm not talking about the case, when the process which is sending
SIGKILL has no write access to cgroupfs.
> Why I am starting to argue... The ability to kill a frozen task complicates
> the code, and since cgroup_enter_stopped() (in this version at least) doesn't
> properly interacts with freezable_schedule() leads to other problems.
>
> From 7/7:
>
> + cgroup.freeze
> + A read-write single value file which exists on non-root cgroups.
> + Allowed values are "0" and "1". The default is "0".
> +
> + Writing "1" to the file causes freezing of the cgroup and all
> + descendant cgroups. This means that all belonging processes will
> + be stopped and will not run until the cgroup will be explicitly
> + unfrozen. Freezing of the cgroup may take some time;
> ^^^^^^^^^^^^^^^^^^
> it may take infinite time.
>
> Just suppose that a task does vfork() and this races with cgroup_do_freeze(true).
> If the new child notices JOBCTL_TRAP_FREEZE before exit/exec the cgroup will be
> never frozen.
Hm, why? cgroup_update_frozen() called from cgroup_post_fork() should bring
the cgroup into the frozen state. If it's not true (I'm missing some race here),
it's a bug, but I don't see why it's not possible in general.
>
> If I read the current kernel/cgroup/freezer.c correctly, CGROUP_FREEZING should
> "always" work (unless a task hangs in D state) and to me this looks more important
> than kill/ptrace support...
Again, I don't see a case, when cgroup v1 freezer will work and the proposed
v2 freezer won't work in general.
>
> > there is no separate controller, which has to be enabled, etc.
>
> agreed, this is nice.
Thanks!
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v8 0/7] freezer for cgroup v2
2019-02-20 22:00 ` Roman Gushchin
@ 2019-02-21 16:29 ` Oleg Nesterov
2019-02-21 17:34 ` Tejun Heo
2019-02-21 22:43 ` Roman Gushchin
0 siblings, 2 replies; 21+ messages in thread
From: Oleg Nesterov @ 2019-02-21 16:29 UTC (permalink / raw)
To: Roman Gushchin
Cc: Roman Gushchin, Tejun Heo, Kernel Team, cgroups, linux-kernel
On 02/20, Roman Gushchin wrote:
>
> On Wed, Feb 20, 2019 at 03:37:48PM +0100, Oleg Nesterov wrote:
> >
> > I tried to not argue with intent, but to be honest I am more and more
> > sceptical... Lets forget about ptrace for the moment.
> >
> > Once again, why do we want a killable freezer?
> >
> > If a user wants to kill a frozen task from CGRP_FROZEN cgroup he can simply
> >
> > 1. send SIGKILL to that task
> >
> > 2. migrate it to the root cgroup.
> >
> > why this doesn't / can't work?
>
> It does work, but it doesn't look as a nice interface to take into
> the cgroup v2 world.
>
> It just not clear, why killing a frozen task requires some cgroup-level
> operations? It doesn't add anything except some additional complexity
> to the userspace.
Yes.
But to me this is a reasonable trade-off because this way we do not add
additional complexity to the kernel.
Actually, "killable" is not that difficult afaics. "ptraceable" looks more
problematic to me. Again, user-space can do
1. PTRACE_SEIZE
2. move the tracee to the root cgroup
3. do anything with the tracee
4. move it back
> Generally speaking, any process hanging in D-state
> for a long time isn't the nicest object from the userspace's point of view.
Roman, this is unfair comparison ;)
> Exactly as a SIGSTOPped process can be killed without sending SIGCONT,
> why a frozen task would require some additional operations?
this too,
> And I'm not talking about the case, when the process which is sending
> SIGKILL has no write access to cgroupfs.
True.
But there is another case. If admin wants to freeze a cgroup then it is not
clear why a user which can send SIGKILL to a frozen process should wake it up.
------------------------------------------------------------------------------
Again, it is not that I hate the idea of killable/ptraceable freezer. Just I
personally think it's not worth the trouble. Perhaps I am wrong, but so far
I do not see a good implementation...
And, apart from reading/writing the registers, what can ptrace do with a frozen
tracee? This doesn't look like a "must have" feature to me.
At least, may I ask you again to make (if possible) a separate patch which adds
the ability to kill/ptrace?
------------------------------------------------------------------------------
> > Why I am starting to argue... The ability to kill a frozen task complicates
> > the code, and since cgroup_enter_stopped() (in this version at least) doesn't
> > properly interacts with freezable_schedule() leads to other problems.
> >
> > From 7/7:
> >
> > + cgroup.freeze
> > + A read-write single value file which exists on non-root cgroups.
> > + Allowed values are "0" and "1". The default is "0".
> > +
> > + Writing "1" to the file causes freezing of the cgroup and all
> > + descendant cgroups. This means that all belonging processes will
> > + be stopped and will not run until the cgroup will be explicitly
> > + unfrozen. Freezing of the cgroup may take some time;
> > ^^^^^^^^^^^^^^^^^^
> > it may take infinite time.
> >
> > Just suppose that a task does vfork() and this races with cgroup_do_freeze(true).
> > If the new child notices JOBCTL_TRAP_FREEZE before exit/exec the cgroup will be
> > never frozen.
>
> Hm, why? cgroup_update_frozen() called from cgroup_post_fork() should bring
> the cgroup into the frozen state. If it's not true (I'm missing some race here),
> it's a bug, but I don't see why it's not possible in general.
A task P calls vfork() and creates the new child C. Now, how can the parent P
(which sleeps in TASK_KILLABLE) call cgroup_enter_stopped() ? It can't until C
exits or execs. C can't exit or exec because it is frozen.
> > If I read the current kernel/cgroup/freezer.c correctly, CGROUP_FREEZING should
> > "always" work (unless a task hangs in D state) and to me this looks more important
> > than kill/ptrace support...
>
> Again, I don't see a case, when cgroup v1 freezer will work and the proposed
> v2 freezer won't work in general.
See above.
Oleg.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v8 0/7] freezer for cgroup v2
2019-02-21 16:29 ` Oleg Nesterov
@ 2019-02-21 17:34 ` Tejun Heo
2019-02-22 16:34 ` Oleg Nesterov
2019-02-21 22:43 ` Roman Gushchin
1 sibling, 1 reply; 21+ messages in thread
From: Tejun Heo @ 2019-02-21 17:34 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Roman Gushchin, Roman Gushchin, Kernel Team, cgroups, linux-kernel
Hello, Oleg.
On Thu, Feb 21, 2019 at 05:29:24PM +0100, Oleg Nesterov wrote:
> But to me this is a reasonable trade-off because this way we do not add
> additional complexity to the kernel.
So, I really wanna avoid allowing userspace to cause D state sleeps.
It's not impossible to work around but becomes really nasty. For
example, imagine a memory pressure based userspace oom handler issuer
kills based on per-cgroup pressure metric (as oomd would do). It
might not necessarily have the insight that a victim cgroup is frozen
or whether it can move out its members to a different cgroup (in a lot
of cases that will cause a lot of confusion in management software).
The frozen state in cgroup1 is a new task state which is different
from all others and in a nasty way and it has been causing various
confusions and mistakes from its users. We really should make it
closer to the existing stop behaviors even if that means more
complexity in the implementation.
> Actually, "killable" is not that difficult afaics. "ptraceable" looks more
> problematic to me. Again, user-space can do
>
> 1. PTRACE_SEIZE
> 2. move the tracee to the root cgroup
> 3. do anything with the tracee
> 4. move it back
which is fine. The goal isn't trying to block userspace from doing
things that it explicitly wants to do. That's fine. We just want
things like killing and ptracing to behave similarly to other stopped
states (ie. avoid introducing completely new behaviors).
> But there is another case. If admin wants to freeze a cgroup then it is not
> clear why a user which can send SIGKILL to a frozen process should wake it up.
>
> ------------------------------------------------------------------------------
> Again, it is not that I hate the idea of killable/ptraceable freezer. Just I
> personally think it's not worth the trouble. Perhaps I am wrong, but so far
> I do not see a good implementation...
>
> And, apart from reading/writing the registers, what can ptrace do with a frozen
> tracee? This doesn't look like a "must have" feature to me.
>
> At least, may I ask you again to make (if possible) a separate patch which adds
> the ability to kill/ptrace?
ptrace support is a lot less important than kill for sure but if at
all possible I think it'd be better to have it because that makes the
frozen state closer to other stopped states and thus less surprising.
To summarize, the ideal result is the frozen state to be "stuck in
jobctl stop loop" but except for looping in it no different from
regular jobctl stop. Plus, register state examination is already
useful for frozen cgroups for debbugging purposes in itself.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v8 0/7] freezer for cgroup v2
2019-02-21 17:34 ` Tejun Heo
@ 2019-02-22 16:34 ` Oleg Nesterov
2019-02-22 18:17 ` Tejun Heo
0 siblings, 1 reply; 21+ messages in thread
From: Oleg Nesterov @ 2019-02-22 16:34 UTC (permalink / raw)
To: Tejun Heo
Cc: Roman Gushchin, Roman Gushchin, Kernel Team, cgroups, linux-kernel
Hi,
On 02/21, Tejun Heo wrote:
>
> So, I really wanna avoid allowing userspace to cause D state sleeps.
...
> ptrace support is a lot less important than kill for sure but if at
> all possible I think it'd be better to have it
Tejun, I agree it would be better. I did not argue with that.
The question is how this can be implemented. And how much uglifications^W
complications in the core kernel code this needs.
> To summarize, the ideal result is the frozen state to be "stuck in
> jobctl stop loop"
Not sure I understand, but I don't think this can work...
Let me repeat, imo the freezer should be "reliable", it shouldn't stuck
in CGRP_FREEZE state forever if, say, it races with vfork(). And personally
I think this is more important than (very limited) ptrace support.
So I think it too should somehow interact with freezable_schedule/etc.
Oleg.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v8 0/7] freezer for cgroup v2
2019-02-22 16:34 ` Oleg Nesterov
@ 2019-02-22 18:17 ` Tejun Heo
2019-02-25 15:57 ` Oleg Nesterov
0 siblings, 1 reply; 21+ messages in thread
From: Tejun Heo @ 2019-02-22 18:17 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Roman Gushchin, Roman Gushchin, Kernel Team, cgroups, linux-kernel
Hey, Oleg.
On Fri, Feb 22, 2019 at 05:34:42PM +0100, Oleg Nesterov wrote:
> > ptrace support is a lot less important than kill for sure but if at
> > all possible I think it'd be better to have it
>
> Tejun, I agree it would be better. I did not argue with that.
>
> The question is how this can be implemented. And how much uglifications^W
> complications in the core kernel code this needs.
Yeah, sure thing. It's always a trade-off but given that this is
something we're gonna expose to userspace as an API which will be
difficult to change too noticeably once released, I think it'd be
worthwhile to spend some effort to get the visible semantics right.
> > To summarize, the ideal result is the frozen state to be "stuck in
> > jobctl stop loop"
>
> Not sure I understand, but I don't think this can work...
>
> Let me repeat, imo the freezer should be "reliable", it shouldn't stuck
> in CGRP_FREEZE state forever if, say, it races with vfork(). And personally
> I think this is more important than (very limited) ptrace support.
Absolutely, it makes no sense to ship it otherwise. I don't think
anyone is saying that we can ship it while it doesn't work reliably.
> So I think it too should somehow interact with freezable_schedule/etc.
You mean freezer_do_not_count(), right? As long as the task is
guaranteed to be trapped by signal stop afterwards (and they are), we
likely can use them the same way. The only thing to be careful about
would be ensuring that we don't end up flipping group level frozen
state inbetween. Would something like that work?
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v8 0/7] freezer for cgroup v2
2019-02-22 18:17 ` Tejun Heo
@ 2019-02-25 15:57 ` Oleg Nesterov
2019-03-05 17:27 ` Tejun Heo
0 siblings, 1 reply; 21+ messages in thread
From: Oleg Nesterov @ 2019-02-25 15:57 UTC (permalink / raw)
To: Tejun Heo
Cc: Roman Gushchin, Roman Gushchin, Kernel Team, cgroups, linux-kernel
On 02/22, Tejun Heo wrote:
>
> > So I think it too should somehow interact with freezable_schedule/etc.
>
> You mean freezer_do_not_count(), right?
yes,
> As long as the task is
> guaranteed to be trapped by signal stop afterwards (and they are), we
> likely can use them the same way. The only thing to be careful about
> would be ensuring that we don't end up flipping group level frozen
> state inbetween. Would something like that work?
I have no idea because I do not understand what exactly do you mean ;)
However. Thinking more about this, I am not sure my concerns were valid.
Yes, cg freezer can "hang" if it races with vfork(). But probably we should
blame vfork(), not freezer.
The problem is, even ^Z can "hang" if the foreground process does vfork()
and the new child stops before exit/exec. Now I recall that I even tried
to make a patch to fix this using ERESTART_RESTARTBLOCK, but had some nasty
problems with blocked signals...
de_thread() should use freezable_schedule() in TASK_KILLABLE too. Currently
it doesn't, but only because we have other (much more serious) problems with
cred_guard_mutex/exec. However, this is is fine wrt cg freezer, other threads
can't be frozen exactly because it is killable.
Anything else does freezer_do_not_count() in TASK_KILLABLE and waits for
another freezable process?
So it seems I have to take my words back, perhaps we can forget about
freezable_schedule/etc.
Oleg.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v8 0/7] freezer for cgroup v2
2019-02-25 15:57 ` Oleg Nesterov
@ 2019-03-05 17:27 ` Tejun Heo
0 siblings, 0 replies; 21+ messages in thread
From: Tejun Heo @ 2019-03-05 17:27 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Roman Gushchin, Roman Gushchin, Kernel Team, cgroups, linux-kernel
Hello, Oleg.
Sorry about the delay.
On Mon, Feb 25, 2019 at 04:57:25PM +0100, Oleg Nesterov wrote:
> > As long as the task is
> > guaranteed to be trapped by signal stop afterwards (and they are), we
> > likely can use them the same way. The only thing to be careful about
> > would be ensuring that we don't end up flipping group level frozen
> > state inbetween. Would something like that work?
>
> I have no idea because I do not understand what exactly do you mean ;)
Heh, sorry about that. What I meant was that we can consider a task
which is blocked in vfork wait as already frozen and that if we do so
we need to be careful so that frozen state doesn't do a flip between
vfork wait ending and the task getting parked again in a jobctl stop.
> However. Thinking more about this, I am not sure my concerns were valid.
> Yes, cg freezer can "hang" if it races with vfork(). But probably we should
> blame vfork(), not freezer.
I think we'd need to cover that ground regardless of where blame lies.
It's weird if freezing doesn't complete cuz one of the tasks messed up
while vforking.
> The problem is, even ^Z can "hang" if the foreground process does vfork()
> and the new child stops before exit/exec. Now I recall that I even tried
> to make a patch to fix this using ERESTART_RESTARTBLOCK, but had some nasty
> problems with blocked signals...
Ugh... yeah, these wait non-interruptible wait sites which can be
exposed to userspace are nasty. They end up adding a unique wait
state visible to userspace which comes with a bunch of corner cases.
> de_thread() should use freezable_schedule() in TASK_KILLABLE too. Currently
> it doesn't, but only because we have other (much more serious) problems with
> cred_guard_mutex/exec. However, this is is fine wrt cg freezer, other threads
> can't be frozen exactly because it is killable.
>
> Anything else does freezer_do_not_count() in TASK_KILLABLE and waits for
> another freezable process?
Can't find any. Hopefully, that was it?
> So it seems I have to take my words back, perhaps we can forget about
> freezable_schedule/etc.
I think it'd be great to be able to handle these if at all possible.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v8 0/7] freezer for cgroup v2
2019-02-21 16:29 ` Oleg Nesterov
2019-02-21 17:34 ` Tejun Heo
@ 2019-02-21 22:43 ` Roman Gushchin
2019-02-22 17:04 ` Oleg Nesterov
1 sibling, 1 reply; 21+ messages in thread
From: Roman Gushchin @ 2019-02-21 22:43 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Roman Gushchin, Tejun Heo, Kernel Team, cgroups, linux-kernel
On Thu, Feb 21, 2019 at 05:29:24PM +0100, Oleg Nesterov wrote:
> On 02/20, Roman Gushchin wrote:
> >
> > On Wed, Feb 20, 2019 at 03:37:48PM +0100, Oleg Nesterov wrote:
> > >
> > > I tried to not argue with intent, but to be honest I am more and more
> > > sceptical... Lets forget about ptrace for the moment.
> > >
> > > Once again, why do we want a killable freezer?
> > >
> > > If a user wants to kill a frozen task from CGRP_FROZEN cgroup he can simply
> > >
> > > 1. send SIGKILL to that task
> > >
> > > 2. migrate it to the root cgroup.
> > >
> > > why this doesn't / can't work?
> >
> > It does work, but it doesn't look as a nice interface to take into
> > the cgroup v2 world.
> >
> > It just not clear, why killing a frozen task requires some cgroup-level
> > operations? It doesn't add anything except some additional complexity
> > to the userspace.
>
> Yes.
>
> But to me this is a reasonable trade-off because this way we do not add
> additional complexity to the kernel.
>
> Actually, "killable" is not that difficult afaics. "ptraceable" looks more
> problematic to me. Again, user-space can do
>
> 1. PTRACE_SEIZE
> 2. move the tracee to the root cgroup
> 3. do anything with the tracee
> 4. move it back
>
> > Generally speaking, any process hanging in D-state
> > for a long time isn't the nicest object from the userspace's point of view.
>
> Roman, this is unfair comparison ;)
Why not? This is exactly the point, with v1 freezer you get a task in D state,
which isn't manageable by userspace without some actions with sysfs.
>
> > Exactly as a SIGSTOPped process can be killed without sending SIGCONT,
> > why a frozen task would require some additional operations?
>
> this too,
>
> > And I'm not talking about the case, when the process which is sending
> > SIGKILL has no write access to cgroupfs.
>
> True.
>
> But there is another case. If admin wants to freeze a cgroup then it is not
> clear why a user which can send SIGKILL to a frozen process should wake it up.
But it will woken up only for a short moment. And the cgroup will remain frozen
in a sense that belonging processes do not consume CPU.
Actually, a signal can be useful: for example, if it was the last process,
the container management software might want to delete the cgroup.
>
> ------------------------------------------------------------------------------
> Again, it is not that I hate the idea of killable/ptraceable freezer. Just I
> personally think it's not worth the trouble. Perhaps I am wrong, but so far
> I do not see a good implementation...
>
> And, apart from reading/writing the registers, what can ptrace do with a frozen
> tracee? This doesn't look like a "must have" feature to me.
I think the minimal requirement is that the tracing application should not hang
and wait for tracee to be unfrozen.
So, imagine you're trying to debug an application in production with gdb,
and occasionally gdb just hangs because some cluster management stuff froze
the tracee's cgroup. Not the best user experience.
>
> At least, may I ask you again to make (if possible) a separate patch which adds
> the ability to kill/ptrace?
I'll try, but not sure if it can make the code easier for review.
It looks like this ability defines the implementation.
> ------------------------------------------------------------------------------
>
> > > Why I am starting to argue... The ability to kill a frozen task complicates
> > > the code, and since cgroup_enter_stopped() (in this version at least) doesn't
> > > properly interacts with freezable_schedule() leads to other problems.
> > >
> > > From 7/7:
> > >
> > > + cgroup.freeze
> > > + A read-write single value file which exists on non-root cgroups.
> > > + Allowed values are "0" and "1". The default is "0".
> > > +
> > > + Writing "1" to the file causes freezing of the cgroup and all
> > > + descendant cgroups. This means that all belonging processes will
> > > + be stopped and will not run until the cgroup will be explicitly
> > > + unfrozen. Freezing of the cgroup may take some time;
> > > ^^^^^^^^^^^^^^^^^^
> > > it may take infinite time.
> > >
> > > Just suppose that a task does vfork() and this races with cgroup_do_freeze(true).
> > > If the new child notices JOBCTL_TRAP_FREEZE before exit/exec the cgroup will be
> > > never frozen.
> >
> > Hm, why? cgroup_update_frozen() called from cgroup_post_fork() should bring
> > the cgroup into the frozen state. If it's not true (I'm missing some race here),
> > it's a bug, but I don't see why it's not possible in general.
>
> A task P calls vfork() and creates the new child C. Now, how can the parent P
> (which sleeps in TASK_KILLABLE) call cgroup_enter_stopped() ? It can't until C
> exits or execs. C can't exit or exec because it is frozen.
Got it. I'll address it in the next version.
Thanks!
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v8 0/7] freezer for cgroup v2
2019-02-21 22:43 ` Roman Gushchin
@ 2019-02-22 17:04 ` Oleg Nesterov
0 siblings, 0 replies; 21+ messages in thread
From: Oleg Nesterov @ 2019-02-22 17:04 UTC (permalink / raw)
To: Roman Gushchin
Cc: Roman Gushchin, Tejun Heo, Kernel Team, cgroups, linux-kernel
On 02/21, Roman Gushchin wrote:
>
> > > Generally speaking, any process hanging in D-state
> > > for a long time isn't the nicest object from the userspace's point of view.
> >
> > Roman, this is unfair comparison ;)
>
> Why not?
OK, you are trolling me, let me troll you back...
So, generally speaking, the very idea of freezer looks wrong, any process
hanging in do_freezer_trap() for a long time isn't the nicest object from
the userspace's point of view.
> > And, apart from reading/writing the registers, what can ptrace do with a frozen
> > tracee? This doesn't look like a "must have" feature to me.
>
> I think the minimal requirement is that the tracing application should not hang
> and wait for tracee to be unfrozen.
> So, imagine you're trying to debug an application in production with gdb,
> and occasionally gdb just hangs because some cluster management stuff froze
> the tracee's cgroup. Not the best user experience.
Firstly, gdb will likely hang anyway. Say, single-step will hang and ^C won't work.
Secondly, just imagine you're trying to debug an application in production with gdb,
and occasionally gdb just hangs because some cluster management stuff froze the
gdb's cgroup. Not the best user experience.
Roman, may be it was not clear, but I never said that ptrace/kill makes no sense.
But yes, we probably disagree about how much this is important. I won't really
argue, but so far I am not sure I understand how this can be implemented.
> > At least, may I ask you again to make (if possible) a separate patch which adds
> > the ability to kill/ptrace?
>
> I'll try, but not sure if it can make the code easier for review.
> It looks like this ability defines the implementation.
OK, I won't insist, I understand that this is not simple.
Oleg.
^ permalink raw reply [flat|nested] 21+ messages in thread