Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1427226 > unrolled thread
| Started by | Kenny Yu <kennyyu@fb.com> |
|---|---|
| First post | 2016-06-21 05:20 +0200 |
| Last post | 2016-06-21 18:30 +0200 |
| Articles | 10 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH] cgroup: Add pids controller event when fork fails because of pid limit Kenny Yu <kennyyu@fb.com> - 2016-06-21 05:20 +0200
Re: [PATCH] cgroup: Add pids controller event when fork fails because of pid limit Johannes Weiner <hannes@cmpxchg.org> - 2016-06-21 06:50 +0200
Re: [PATCH v2] cgroup: Add pids controller event when fork fails because of pid limit Aleksa Sarai <cyphar@cyphar.com> - 2016-06-21 18:10 +0200
Re: [PATCH v2] cgroup: Add pids controller event when fork fails because of pid limit Tejun Heo <tj@kernel.org> - 2016-06-21 18:40 +0200
[PATCH v3] cgroup: Add pids controller event when fork fails because of pid limit Kenny Yu <kennyyu@fb.com> - 2016-06-21 19:00 +0200
Re: [PATCH v3] cgroup: Add pids controller event when fork fails because of pid limit Tejun Heo <tj@kernel.org> - 2016-06-21 19:20 +0200
Re: [PATCH v3] cgroup: Add pids controller event when fork fails because of pid limit Tejun Heo <tj@kernel.org> - 2016-06-21 20:30 +0200
[PATCH v4] cgroup: Add pids controller event when fork fails because of pid limit Kenny Yu <kennyyu@fb.com> - 2016-06-21 20:40 +0200
Re: [PATCH v3] cgroup: Add pids controller event when fork fails because of pid limit Kenny Yu <kennyyu@fb.com> - 2016-06-21 20:40 +0200
[PATCH v2] cgroup: Add pids controller event when fork fails because of pid limit Kenny Yu <kennyyu@fb.com> - 2016-06-21 18:30 +0200
| From | Kenny Yu <kennyyu@fb.com> |
|---|---|
| Date | 2016-06-21 05:20 +0200 |
| Subject | [PATCH] cgroup: Add pids controller event when fork fails because of pid limit |
| Message-ID | <rMkoh-cA-1@gated-at.bofh.it> |
Summary:
This patch adds more visibility into the pids controller when the controller
rejects a fork request. Whenever fork fails because the limit on the number of
pids in the cgroup is reached, the controller will log this and also notify the
newly added cgroups events file. The `max` key in the events file represents
the number of times fork failed because of the pids controller.
This change also adds an atomic boolean to prevent logging too much (e.g. a fork
bomb). The message is logged once per cgroup until the next time the pids limit
changes.
Signed-off-by: Kenny Yu <kennyyu@fb.com>
---
kernel/cgroup_pids.c | 46 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 45 insertions(+), 1 deletion(-)
diff --git a/kernel/cgroup_pids.c b/kernel/cgroup_pids.c
index b93cbe3..8ba8c8f 100644
--- a/kernel/cgroup_pids.c
+++ b/kernel/cgroup_pids.c
@@ -49,6 +49,18 @@ struct pids_cgroup {
*/
atomic64_t counter;
int64_t limit;
+
+ /* Handle for "pids.events" */
+ struct cgroup_file events_file;
+
+ /* Number of times fork failed because limit was hit. */
+ atomic64_t events_limit;
+
+ /*
+ * To avoid logging too much (e.g. during a fork bomb), log only once
+ * per cgroup and reset this when the limit changes.
+ */
+ atomic_t events_limit_logged;
};
static struct pids_cgroup *css_pids(struct cgroup_subsys_state *css)
@@ -72,6 +84,8 @@ pids_css_alloc(struct cgroup_subsys_state *parent)
pids->limit = PIDS_MAX;
atomic64_set(&pids->counter, 0);
+ atomic64_set(&pids->events_limit, 0);
+ atomic_set(&pids->events_limit_logged, 0);
return &pids->css;
}
@@ -205,6 +219,17 @@ static void pids_cancel_attach(struct cgroup_taskset *tset)
}
}
+static void pids_fork_failed_event(struct pids_cgroup *pids)
+{
+ atomic64_inc(&pids->events_limit);
+ cgroup_file_notify(&pids->events_file);
+ if (!atomic_xchg(&pids->events_limit_logged, 1)) {
+ pr_info("cgroup: fork rejected by pids controller in ");
+ pr_cont_cgroup_path(task_cgroup(current, pids_cgrp_id));
+ pr_cont("\n");
+ }
+}
+
/*
* task_css_check(true) in pids_can_fork() and pids_cancel_fork() relies
* on threadgroup_change_begin() held by the copy_process().
@@ -213,10 +238,14 @@ static int pids_can_fork(struct task_struct *task)
{
struct cgroup_subsys_state *css;
struct pids_cgroup *pids;
+ int err;
css = task_css_check(current, pids_cgrp_id, true);
pids = css_pids(css);
- return pids_try_charge(pids, 1);
+ err = pids_try_charge(pids, 1);
+ if (err)
+ pids_fork_failed_event(pids);
+ return err;
}
static void pids_cancel_fork(struct task_struct *task)
@@ -263,6 +292,7 @@ set_limit:
* critical that any racing fork()s follow the new limit.
*/
pids->limit = limit;
+ atomic_set(&pids->events_limit_logged, 0);
return nbytes;
}
@@ -288,6 +318,14 @@ static s64 pids_current_read(struct cgroup_subsys_state *css,
return atomic64_read(&pids->counter);
}
+static int pids_events_show(struct seq_file *sf, void *v)
+{
+ struct pids_cgroup *pids = css_pids(seq_css(sf));
+
+ seq_printf(sf, "max %ld\n", atomic64_read(&pids->events_limit));
+ return 0;
+}
+
static struct cftype pids_files[] = {
{
.name = "max",
@@ -299,6 +337,12 @@ static struct cftype pids_files[] = {
.name = "current",
.read_s64 = pids_current_read,
},
+ {
+ .name = "events",
+ .seq_show = pids_events_show,
+ .file_offset = offsetof(struct pids_cgroup, events_file),
+ .flags = CFTYPE_NOT_ON_ROOT,
+ },
{ } /* terminate */
};
--
2.8.0.rc2
[toc] | [next] | [standalone]
| From | Johannes Weiner <hannes@cmpxchg.org> |
|---|---|
| Date | 2016-06-21 06:50 +0200 |
| Subject | Re: [PATCH] cgroup: Add pids controller event when fork fails because of pid limit |
| Message-ID | <rMlNn-12h-1@gated-at.bofh.it> |
| In reply to | #1427226 |
On Mon, Jun 20, 2016 at 08:09:22PM -0700, Kenny Yu wrote:
> Summary:
> This patch adds more visibility into the pids controller when the controller
> rejects a fork request. Whenever fork fails because the limit on the number of
> pids in the cgroup is reached, the controller will log this and also notify the
> newly added cgroups events file. The `max` key in the events file represents
> the number of times fork failed because of the pids controller.
>
> This change also adds an atomic boolean to prevent logging too much (e.g. a fork
> bomb). The message is logged once per cgroup until the next time the pids limit
> changes.
>
> Signed-off-by: Kenny Yu <kennyyu@fb.com>
This makes sense to me. Hitting the cgroup PID limit right now is
somewhat ominous. A little more visibility would help.
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
One comment below, but mostly a matter of preference:
> @@ -205,6 +219,17 @@ static void pids_cancel_attach(struct cgroup_taskset *tset)
> }
> }
>
> +static void pids_fork_failed_event(struct pids_cgroup *pids)
> +{
> + atomic64_inc(&pids->events_limit);
> + cgroup_file_notify(&pids->events_file);
> + if (!atomic_xchg(&pids->events_limit_logged, 1)) {
> + pr_info("cgroup: fork rejected by pids controller in ");
> + pr_cont_cgroup_path(task_cgroup(current, pids_cgrp_id));
> + pr_cont("\n");
> + }
> +}
> +
> /*
> * task_css_check(true) in pids_can_fork() and pids_cancel_fork() relies
> * on threadgroup_change_begin() held by the copy_process().
> @@ -213,10 +238,14 @@ static int pids_can_fork(struct task_struct *task)
> {
> struct cgroup_subsys_state *css;
> struct pids_cgroup *pids;
> + int err;
>
> css = task_css_check(current, pids_cgrp_id, true);
> pids = css_pids(css);
> - return pids_try_charge(pids, 1);
> + err = pids_try_charge(pids, 1);
> + if (err)
> + pids_fork_failed_event(pids);
That function call/name seems somewhat clunky. Maybe it would be
better to inline its body directly into pids_try_charge() before
return -EAGAIN?
[toc] | [prev] | [next] | [standalone]
| From | Aleksa Sarai <cyphar@cyphar.com> |
|---|---|
| Date | 2016-06-21 18:10 +0200 |
| Subject | Re: [PATCH v2] cgroup: Add pids controller event when fork fails because of pid limit |
| Message-ID | <rMwpr-853-3@gated-at.bofh.it> |
| In reply to | #1427286 |
> @@ -213,10 +227,21 @@ static int pids_can_fork(struct task_struct *task)
> {
> struct cgroup_subsys_state *css;
> struct pids_cgroup *pids;
> + int err;
>
> css = task_css_check(current, pids_cgrp_id, true);
> pids = css_pids(css);
> - return pids_try_charge(pids, 1);
> + err = pids_try_charge(pids, 1);
> + if (err) {
> + atomic64_inc(&pids->events_limit);
> + cgroup_file_notify(&pids->events_file);
> + if (!atomic_xchg(&pids->events_limit_logged, 1)) {
> + pr_info("cgroup: fork rejected by pids controller in ");
> + pr_cont_cgroup_path(task_cgroup(current, pids_cgrp_id));
> + pr_cont("\n");
> + }
> + }
> + return err;
> }
Why are we logging this? Isn't the pids.events file enough
information? I feel like you could remove a lot of logic if you don't
log this.
And even if we do end up logging it, why have the boolean flag (the
counter always increases, just log if the counter is currently 0 and
you're incrementing it).
--
Aleksa Sarai (cyphar)
www.cyphar.com
[toc] | [prev] | [next] | [standalone]
| From | Tejun Heo <tj@kernel.org> |
|---|---|
| Date | 2016-06-21 18:40 +0200 |
| Subject | Re: [PATCH v2] cgroup: Add pids controller event when fork fails because of pid limit |
| Message-ID | <rMwSu-8eX-31@gated-at.bofh.it> |
| In reply to | #1427908 |
Hello, On Wed, Jun 22, 2016 at 02:07:09AM +1000, Aleksa Sarai wrote: > Why are we logging this? Isn't the pids.events file enough > information? I feel like you could remove a lot of logic if you don't > log this. I think logging it is a good idea. People aren't used to think about the pids controller when fork fails and I've seen people getting royally confused by it. Also, if fork is being rejected on the right (or wrong) cgroup, investigating why that's happening can be extremely challenging (e.g. can't login). > And even if we do end up logging it, why have the boolean flag (the > counter always increases, just log if the counter is currently 0 and > you're incrementing it). Ah, that's true. I like the fact that the warning message will be printed after each change to the limit but yeah going off of zero events_limit count should be fine too. Thanks. -- tejun
[toc] | [prev] | [next] | [standalone]
| From | Kenny Yu <kennyyu@fb.com> |
|---|---|
| Date | 2016-06-21 19:00 +0200 |
| Subject | [PATCH v3] cgroup: Add pids controller event when fork fails because of pid limit |
| Message-ID | <rMxbQ-8nf-11@gated-at.bofh.it> |
| In reply to | #1427943 |
Summary:
This patch adds more visibility into the pids controller when the controller
rejects a fork request. Whenever fork fails because the limit on the number of
pids in the cgroup is reached, the controller will log this and also notify the
newly added cgroups events file. The `max` key in the events file represents
the number of times fork failed because of the pids controller.
This change also adds an atomic boolean to prevent logging too much (e.g. a fork
bomb). The message is logged once per cgroup until the next time the pids limit
changes.
Signed-off-by: Kenny Yu <kennyyu@fb.com>
Acked-by: Johannes Weiner <hannes <at> cmpxchg.org>
---
kernel/cgroup_pids.c | 36 +++++++++++++++++++++++++++++++++++-
1 file changed, 35 insertions(+), 1 deletion(-)
diff --git a/kernel/cgroup_pids.c b/kernel/cgroup_pids.c
index b93cbe3..74cb0fe 100644
--- a/kernel/cgroup_pids.c
+++ b/kernel/cgroup_pids.c
@@ -49,6 +49,12 @@ struct pids_cgroup {
*/
atomic64_t counter;
int64_t limit;
+
+ /* Handle for "pids.events" */
+ struct cgroup_file events_file;
+
+ /* Number of times fork failed because limit was hit. */
+ atomic64_t events_limit;
};
static struct pids_cgroup *css_pids(struct cgroup_subsys_state *css)
@@ -72,6 +78,7 @@ pids_css_alloc(struct cgroup_subsys_state *parent)
pids->limit = PIDS_MAX;
atomic64_set(&pids->counter, 0);
+ atomic64_set(&pids->events_limit, 0);
return &pids->css;
}
@@ -213,10 +220,23 @@ static int pids_can_fork(struct task_struct *task)
{
struct cgroup_subsys_state *css;
struct pids_cgroup *pids;
+ int err;
+ int events_limit;
css = task_css_check(current, pids_cgrp_id, true);
pids = css_pids(css);
- return pids_try_charge(pids, 1);
+ err = pids_try_charge(pids, 1);
+ if (err) {
+ events_limit = atomic64_inc_return(&pids->events_limit);
+ cgroup_file_notify(&pids->events_file);
+ /* Only log the first time events_limit is incremented. */
+ if (events_limit == 1) {
+ pr_info("cgroup: fork rejected by pids controller in ");
+ pr_cont_cgroup_path(task_cgroup(current, pids_cgrp_id));
+ pr_cont("\n");
+ }
+ }
+ return err;
}
static void pids_cancel_fork(struct task_struct *task)
@@ -288,6 +308,14 @@ static s64 pids_current_read(struct cgroup_subsys_state *css,
return atomic64_read(&pids->counter);
}
+static int pids_events_show(struct seq_file *sf, void *v)
+{
+ struct pids_cgroup *pids = css_pids(seq_css(sf));
+
+ seq_printf(sf, "max %ld\n", atomic64_read(&pids->events_limit));
+ return 0;
+}
+
static struct cftype pids_files[] = {
{
.name = "max",
@@ -299,6 +327,12 @@ static struct cftype pids_files[] = {
.name = "current",
.read_s64 = pids_current_read,
},
+ {
+ .name = "events",
+ .seq_show = pids_events_show,
+ .file_offset = offsetof(struct pids_cgroup, events_file),
+ .flags = CFTYPE_NOT_ON_ROOT,
+ },
{ } /* terminate */
};
--
2.8.0.rc2
[toc] | [prev] | [next] | [standalone]
| From | Tejun Heo <tj@kernel.org> |
|---|---|
| Date | 2016-06-21 19:20 +0200 |
| Subject | Re: [PATCH v3] cgroup: Add pids controller event when fork fails because of pid limit |
| Message-ID | <rMxvb-ii-5@gated-at.bofh.it> |
| In reply to | #1427956 |
Hello,
Just a couple nits.
On Tue, Jun 21, 2016 at 09:56:38AM -0700, Kenny Yu wrote:
> Summary:
No need for "Summary:" tag.
> This patch adds more visibility into the pids controller when the controller
> rejects a fork request. Whenever fork fails because the limit on the number of
> pids in the cgroup is reached, the controller will log this and also notify the
> newly added cgroups events file. The `max` key in the events file represents
> the number of times fork failed because of the pids controller.
>
> This change also adds an atomic boolean to prevent logging too much (e.g. a fork
> bomb). The message is logged once per cgroup until the next time the pids limit
> changes.
The above paragraph isn't uptodate anymore.
> @@ -213,10 +220,23 @@ static int pids_can_fork(struct task_struct *task)
> {
> struct cgroup_subsys_state *css;
> struct pids_cgroup *pids;
> + int err;
> + int events_limit;
>
> css = task_css_check(current, pids_cgrp_id, true);
> pids = css_pids(css);
> - return pids_try_charge(pids, 1);
> + err = pids_try_charge(pids, 1);
> + if (err) {
> + events_limit = atomic64_inc_return(&pids->events_limit);
> + cgroup_file_notify(&pids->events_file);
> + /* Only log the first time events_limit is incremented. */
> + if (events_limit == 1) {
> + pr_info("cgroup: fork rejected by pids controller in ");
> + pr_cont_cgroup_path(task_cgroup(current, pids_cgrp_id));
> + pr_cont("\n");
> + }
> + }
> + return err;
> }
It'd be better to use atomic64_inc_and_test() instead.
if (err) {
if (atomic64_inc_and_test()) {
pr_xxx...;
}
cgroup_file_notify(&pids->events_file);
}
Thanks.
--
tejun
[toc] | [prev] | [next] | [standalone]
| From | Tejun Heo <tj@kernel.org> |
|---|---|
| Date | 2016-06-21 20:30 +0200 |
| Subject | Re: [PATCH v3] cgroup: Add pids controller event when fork fails because of pid limit |
| Message-ID | <rMyhA-Qq-17@gated-at.bofh.it> |
| In reply to | #1427979 |
Hello,
On Tue, Jun 21, 2016 at 05:23:40PM +0000, Kenny Yu wrote:
> >It'd be better to use atomic64_inc_and_test() instead.
> >
> > if (err) {
> > if (atomic64_inc_and_test()) {
> > pr_xxx...;
> > }
> > cgroup_file_notify(&pids->events_file);
> > }
> >
>
> According to the docs https://www.kernel.org/doc/Documentation/atomic_ops.txt ,
> it looks like atomic_inc_and_test returns "a boolean indicating whether the resulting
> counter value was zero or not", which will only happen when the counter goes from
> negative to 0. I'll keep it as atomic_inc_return and get rid of the temp variable.
Right you're. Sorry about the confusion. Yeah, that sounds good to
me.
Thanks!
--
tejun
[toc] | [prev] | [next] | [standalone]
| From | Kenny Yu <kennyyu@fb.com> |
|---|---|
| Date | 2016-06-21 20:40 +0200 |
| Subject | [PATCH v4] cgroup: Add pids controller event when fork fails because of pid limit |
| Message-ID | <rMyhA-Qq-19@gated-at.bofh.it> |
| In reply to | #1428044 |
This patch adds more visibility into the pids controller when the controller
rejects a fork request. Whenever fork fails because the limit on the number of
pids in the cgroup is reached, the controller will log this and also notify the
newly added cgroups events file. The `max` key in the events file represents
the number of times fork failed because of the pids controller.
This change also logs only the first time the `max` event counter is
incremented. This is to provide a hint to the user to understand why fork
failed, as users are not yet used to seeing fork failures because of the
pids controller.
Signed-off-by: Kenny Yu <kennyyu@fb.com>
Acked-by: Johannes Weiner <hannes <at> cmpxchg.org>
---
kernel/cgroup_pids.c | 34 +++++++++++++++++++++++++++++++++-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/kernel/cgroup_pids.c b/kernel/cgroup_pids.c
index b93cbe3..bd2490b 100644
--- a/kernel/cgroup_pids.c
+++ b/kernel/cgroup_pids.c
@@ -49,6 +49,12 @@ struct pids_cgroup {
*/
atomic64_t counter;
int64_t limit;
+
+ /* Handle for "pids.events" */
+ struct cgroup_file events_file;
+
+ /* Number of times fork failed because limit was hit. */
+ atomic64_t events_limit;
};
static struct pids_cgroup *css_pids(struct cgroup_subsys_state *css)
@@ -72,6 +78,7 @@ pids_css_alloc(struct cgroup_subsys_state *parent)
pids->limit = PIDS_MAX;
atomic64_set(&pids->counter, 0);
+ atomic64_set(&pids->events_limit, 0);
return &pids->css;
}
@@ -213,10 +220,21 @@ static int pids_can_fork(struct task_struct *task)
{
struct cgroup_subsys_state *css;
struct pids_cgroup *pids;
+ int err;
css = task_css_check(current, pids_cgrp_id, true);
pids = css_pids(css);
- return pids_try_charge(pids, 1);
+ err = pids_try_charge(pids, 1);
+ if (err) {
+ /* Only log the first time events_limit is incremented. */
+ if (atomic64_inc_return(&pids->events_limit) == 1) {
+ pr_info("cgroup: fork rejected by pids controller in ");
+ pr_cont_cgroup_path(task_cgroup(current, pids_cgrp_id));
+ pr_cont("\n");
+ }
+ cgroup_file_notify(&pids->events_file);
+ }
+ return err;
}
static void pids_cancel_fork(struct task_struct *task)
@@ -288,6 +306,14 @@ static s64 pids_current_read(struct cgroup_subsys_state *css,
return atomic64_read(&pids->counter);
}
+static int pids_events_show(struct seq_file *sf, void *v)
+{
+ struct pids_cgroup *pids = css_pids(seq_css(sf));
+
+ seq_printf(sf, "max %ld\n", atomic64_read(&pids->events_limit));
+ return 0;
+}
+
static struct cftype pids_files[] = {
{
.name = "max",
@@ -299,6 +325,12 @@ static struct cftype pids_files[] = {
.name = "current",
.read_s64 = pids_current_read,
},
+ {
+ .name = "events",
+ .seq_show = pids_events_show,
+ .file_offset = offsetof(struct pids_cgroup, events_file),
+ .flags = CFTYPE_NOT_ON_ROOT,
+ },
{ } /* terminate */
};
--
2.8.0.rc2
[toc] | [prev] | [next] | [standalone]
| From | Kenny Yu <kennyyu@fb.com> |
|---|---|
| Date | 2016-06-21 20:40 +0200 |
| Subject | Re: [PATCH v3] cgroup: Add pids controller event when fork fails because of pid limit |
| Message-ID | <rMyAV-Xa-1@gated-at.bofh.it> |
| In reply to | #1427979 |
Thanks for the feedback Tejun!
On 6/21/16, 1:12 PM, "Tejun Heo" <htejun@gmail.com on behalf of tj@kernel.org> wrote:
>Hello,
>
>Just a couple nits.
>
>On Tue, Jun 21, 2016 at 09:56:38AM -0700, Kenny Yu wrote:
>> Summary:
>
>No need for "Summary:" tag.
>
>> This patch adds more visibility into the pids controller when the controller
>> rejects a fork request. Whenever fork fails because the limit on the number of
>> pids in the cgroup is reached, the controller will log this and also notify the
>> newly added cgroups events file. The `max` key in the events file represents
>> the number of times fork failed because of the pids controller.
>>
>> This change also adds an atomic boolean to prevent logging too much (e.g. a fork
>> bomb). The message is logged once per cgroup until the next time the pids limit
>> changes.
>
>The above paragraph isn't uptodate anymore.
Thanks! Will change.
>
>> @@ -213,10 +220,23 @@ static int pids_can_fork(struct task_struct *task)
>> {
>> struct cgroup_subsys_state *css;
>> struct pids_cgroup *pids;
>> + int err;
>> + int events_limit;
>>
>> css = task_css_check(current, pids_cgrp_id, true);
>> pids = css_pids(css);
>> - return pids_try_charge(pids, 1);
>> + err = pids_try_charge(pids, 1);
>> + if (err) {
>> + events_limit = atomic64_inc_return(&pids->events_limit);
>> + cgroup_file_notify(&pids->events_file);
>> + /* Only log the first time events_limit is incremented. */
>> + if (events_limit == 1) {
>> + pr_info("cgroup: fork rejected by pids controller in ");
>> + pr_cont_cgroup_path(task_cgroup(current, pids_cgrp_id));
>> + pr_cont("\n");
>> + }
>> + }
>> + return err;
>> }
>
>It'd be better to use atomic64_inc_and_test() instead.
>
> if (err) {
> if (atomic64_inc_and_test()) {
> pr_xxx...;
> }
> cgroup_file_notify(&pids->events_file);
> }
>
According to the docs https://www.kernel.org/doc/Documentation/atomic_ops.txt ,
it looks like atomic_inc_and_test returns "a boolean indicating whether the resulting
counter value was zero or not", which will only happen when the counter goes from
negative to 0. I'll keep it as atomic_inc_return and get rid of the temp variable.
>Thanks.
>
>--
>tejun
Thanks,
Kenny
[toc] | [prev] | [next] | [standalone]
| From | Kenny Yu <kennyyu@fb.com> |
|---|---|
| Date | 2016-06-21 18:30 +0200 |
| Subject | [PATCH v2] cgroup: Add pids controller event when fork fails because of pid limit |
| Message-ID | <rMwpr-853-5@gated-at.bofh.it> |
| In reply to | #1427286 |
Summary:
This patch adds more visibility into the pids controller when the controller
rejects a fork request. Whenever fork fails because the limit on the number of
pids in the cgroup is reached, the controller will log this and also notify the
newly added cgroups events file. The `max` key in the events file represents
the number of times fork failed because of the pids controller.
This change also adds an atomic boolean to prevent logging too much (e.g. a fork
bomb). The message is logged once per cgroup until the next time the pids limit
changes.
Signed-off-by: Kenny Yu <kennyyu@fb.com>
Acked-by: Johannes Weiner <hannes <at> cmpxchg.org>
---
kernel/cgroup_pids.c | 42 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 41 insertions(+), 1 deletion(-)
diff --git a/kernel/cgroup_pids.c b/kernel/cgroup_pids.c
index b93cbe3..412f4d8 100644
--- a/kernel/cgroup_pids.c
+++ b/kernel/cgroup_pids.c
@@ -49,6 +49,18 @@ struct pids_cgroup {
*/
atomic64_t counter;
int64_t limit;
+
+ /* Handle for "pids.events" */
+ struct cgroup_file events_file;
+
+ /* Number of times fork failed because limit was hit. */
+ atomic64_t events_limit;
+
+ /*
+ * To avoid logging too much (e.g. during a fork bomb), log only once
+ * per cgroup and reset this when the limit changes.
+ */
+ atomic_t events_limit_logged;
};
static struct pids_cgroup *css_pids(struct cgroup_subsys_state *css)
@@ -72,6 +84,8 @@ pids_css_alloc(struct cgroup_subsys_state *parent)
pids->limit = PIDS_MAX;
atomic64_set(&pids->counter, 0);
+ atomic64_set(&pids->events_limit, 0);
+ atomic_set(&pids->events_limit_logged, 0);
return &pids->css;
}
@@ -213,10 +227,21 @@ static int pids_can_fork(struct task_struct *task)
{
struct cgroup_subsys_state *css;
struct pids_cgroup *pids;
+ int err;
css = task_css_check(current, pids_cgrp_id, true);
pids = css_pids(css);
- return pids_try_charge(pids, 1);
+ err = pids_try_charge(pids, 1);
+ if (err) {
+ atomic64_inc(&pids->events_limit);
+ cgroup_file_notify(&pids->events_file);
+ if (!atomic_xchg(&pids->events_limit_logged, 1)) {
+ pr_info("cgroup: fork rejected by pids controller in ");
+ pr_cont_cgroup_path(task_cgroup(current, pids_cgrp_id));
+ pr_cont("\n");
+ }
+ }
+ return err;
}
static void pids_cancel_fork(struct task_struct *task)
@@ -263,6 +288,7 @@ set_limit:
* critical that any racing fork()s follow the new limit.
*/
pids->limit = limit;
+ atomic_set(&pids->events_limit_logged, 0);
return nbytes;
}
@@ -288,6 +314,14 @@ static s64 pids_current_read(struct cgroup_subsys_state *css,
return atomic64_read(&pids->counter);
}
+static int pids_events_show(struct seq_file *sf, void *v)
+{
+ struct pids_cgroup *pids = css_pids(seq_css(sf));
+
+ seq_printf(sf, "max %ld\n", atomic64_read(&pids->events_limit));
+ return 0;
+}
+
static struct cftype pids_files[] = {
{
.name = "max",
@@ -299,6 +333,12 @@ static struct cftype pids_files[] = {
.name = "current",
.read_s64 = pids_current_read,
},
+ {
+ .name = "events",
+ .seq_show = pids_events_show,
+ .file_offset = offsetof(struct pids_cgroup, events_file),
+ .flags = CFTYPE_NOT_ON_ROOT,
+ },
{ } /* terminate */
};
--
2.8.0.rc2
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web