oom: Be less verbose

oom: Be less verbose

[Richard Weinberger]

Processes within Linux containers very often hit their memory limits.
This has the side effect that the kernel log gets spammed all day with
useless OOM messages.
If a userspace program listens to the memory cgroup event fd to
get notified upon OOM we can avoid this spamming and be less verbose.

With this patch applied the OOM killer will only print much details
if nobody listens to the affected memory cgroup event fd.
I can also think of a new sysctl like "vm.oom_verbose=1" to guarantee the old
behavior even if we have listeners.

What do you think?

Thanks,
//richard

[RFC][PATCH] oom: Be less verbose if the oom_control event fd has listeners
--
 include/linux/memcontrol.h |    6 ++++++
 mm/memcontrol.c            |   20 ++++++++++++++++++++
 mm/oom_kill.c              |    2 +-
 3 files changed, 27 insertions(+), 1 deletion(-)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[RFC][PATCH] oom: Be less verbose if the oom_control event fd has listeners

[Richard Weinberger]

Don't spam the kernel logs if the oom_control event fd has listeners.
In this case there is no need to print that much lines as user space
will anyway notice that the memory cgroup has reached its limit.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 include/linux/memcontrol.h |  6 ++++++
 mm/memcontrol.c            | 20 ++++++++++++++++++++
 mm/oom_kill.c              |  2 +-
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index b569b8b..506a1a9 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -131,6 +131,7 @@ int mem_cgroup_inactive_anon_is_low(struct lruvec *lruvec);
 int mem_cgroup_select_victim_node(struct mem_cgroup *memcg);
 unsigned long mem_cgroup_get_lru_size(struct lruvec *lruvec, enum lru_list);
 void mem_cgroup_update_lru_size(struct lruvec *, enum lru_list, int);
+extern int mem_cgroup_has_listeners(struct mem_cgroup *memcg);
 extern void mem_cgroup_print_oom_info(struct mem_cgroup *memcg,
 					struct task_struct *p);
 extern void mem_cgroup_replace_page_cache(struct page *oldpage,
@@ -358,6 +359,11 @@ mem_cgroup_update_lru_size(struct lruvec *lruvec, enum lru_list lru,
 {
 }
 
+static inline int mem_cgroup_has_listeners(struct mem_cgroup *memcg)
+{
+	return 0;
+}
+
 static inline void
 mem_cgroup_print_oom_info(struct mem_cgroup *memcg, struct task_struct *p)
 {
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 5177c6d..3864c6a 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -5777,6 +5777,26 @@ static void mem_cgroup_oom_unregister_event(struct mem_cgroup *memcg,
 	spin_unlock(&memcg_oom_lock);
 }
 
+/**
+ * mem_cgroup_has_listeners: Returns true in case we have one ore more
+ * listeners on our oom notifier event fd.
+ * @memcg: The memory cgroup that went over limit
+ */
+int mem_cgroup_has_listeners(struct mem_cgroup *memcg)
+{
+	int ret = 0;
+
+	if (!memcg)
+		goto out;
+
+	spin_lock(&memcg_oom_lock);
+	ret = !list_empty(&memcg->oom_notify);
+	spin_unlock(&memcg_oom_lock);
+
+out:
+	return ret;
+}
+
 static int mem_cgroup_oom_control_read(struct seq_file *sf, void *v)
 {
 	struct mem_cgroup *memcg = mem_cgroup_from_css(seq_css(sf));
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 3291e82..b5c9433 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -434,7 +434,7 @@ void oom_kill_process(struct task_struct *p, gfp_t gfp_mask, int order,
 		return;
 	}
 
-	if (__ratelimit(&oom_rs))
+	if (__ratelimit(&oom_rs) && !mem_cgroup_has_listeners(memcg))
 		dump_header(p, gfp_mask, order, memcg, nodemask);
 
 	task_lock(p);
-- 
1.8.4.2

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC][PATCH] oom: Be less verbose if the oom_control event fd has listeners

[Oleg Nesterov]

On 06/05, Richard Weinberger wrote:
>
> +int mem_cgroup_has_listeners(struct mem_cgroup *memcg)
> +{
> +	int ret = 0;
> +
> +	if (!memcg)
> +		goto out;
> +
> +	spin_lock(&memcg_oom_lock);
> +	ret = !list_empty(&memcg->oom_notify);
> +	spin_unlock(&memcg_oom_lock);
> +
> +out:
> +	return ret;
> +}

Do we really need memcg_oom_lock to check list_empty() ? With or without
this lock we can race with list_add/del anyway, and I guess we do not care.

And perhaps the caller should check memcg != NULL. but this is subjective,
I won't argue.

Oleg.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC][PATCH] oom: Be less verbose if the oom_control event fd has listeners

[Richard Weinberger]

Am 05.06.2014 16:18, schrieb Oleg Nesterov:
> On 06/05, Richard Weinberger wrote:
>>
>> +int mem_cgroup_has_listeners(struct mem_cgroup *memcg)
>> +{
>> +	int ret = 0;
>> +
>> +	if (!memcg)
>> +		goto out;
>> +
>> +	spin_lock(&memcg_oom_lock);
>> +	ret = !list_empty(&memcg->oom_notify);
>> +	spin_unlock(&memcg_oom_lock);
>> +
>> +out:
>> +	return ret;
>> +}
> 
> Do we really need memcg_oom_lock to check list_empty() ? With or without
> this lock we can race with list_add/del anyway, and I guess we do not care.

Hmm, in mm/memcontrol.c all list_dev/add are under memcg_oom_lock.
What do I miss?

Thanks,
//richard

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC][PATCH] oom: Be less verbose if the oom_control event fd has listeners

[Oleg Nesterov]

On 06/05, Richard Weinberger wrote:
>
> Am 05.06.2014 16:18, schrieb Oleg Nesterov:
> > On 06/05, Richard Weinberger wrote:
> >>
> >> +int mem_cgroup_has_listeners(struct mem_cgroup *memcg)
> >> +{
> >> +	int ret = 0;
> >> +
> >> +	if (!memcg)
> >> +		goto out;
> >> +
> >> +	spin_lock(&memcg_oom_lock);
> >> +	ret = !list_empty(&memcg->oom_notify);
> >> +	spin_unlock(&memcg_oom_lock);
> >> +
> >> +out:
> >> +	return ret;
> >> +}
> >
> > Do we really need memcg_oom_lock to check list_empty() ? With or without
> > this lock we can race with list_add/del anyway, and I guess we do not care.
>
> Hmm, in mm/memcontrol.c all list_dev/add are under memcg_oom_lock.

And? How this lock can help to check list_empty() ?

list_add/del can come right after mem_cgroup_has_listeners() and change
the value of list_empty() anyway.

> What do I miss?

Or me...

Oleg.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC][PATCH] oom: Be less verbose if the oom_control event fd has listeners

[Richard Weinberger]

Am 05.06.2014 18:00, schrieb Oleg Nesterov:
> On 06/05, Richard Weinberger wrote:
>>
>> Am 05.06.2014 16:18, schrieb Oleg Nesterov:
>>> On 06/05, Richard Weinberger wrote:
>>>>
>>>> +int mem_cgroup_has_listeners(struct mem_cgroup *memcg)
>>>> +{
>>>> +	int ret = 0;
>>>> +
>>>> +	if (!memcg)
>>>> +		goto out;
>>>> +
>>>> +	spin_lock(&memcg_oom_lock);
>>>> +	ret = !list_empty(&memcg->oom_notify);
>>>> +	spin_unlock(&memcg_oom_lock);
>>>> +
>>>> +out:
>>>> +	return ret;
>>>> +}
>>>
>>> Do we really need memcg_oom_lock to check list_empty() ? With or without
>>> this lock we can race with list_add/del anyway, and I guess we do not care.
>>
>> Hmm, in mm/memcontrol.c all list_dev/add are under memcg_oom_lock.
> 
> And? How this lock can help to check list_empty() ?
> 
> list_add/del can come right after mem_cgroup_has_listeners() and change
> the value of list_empty() anyway.

Ahh, now I can follow your mind. :)

Thanks,
//richard

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC][PATCH] oom: Be less verbose if the oom_control event fd has listeners

[Michal Hocko]

On Thu 05-06-14 16:00:41, Richard Weinberger wrote:
> Don't spam the kernel logs if the oom_control event fd has listeners.
> In this case there is no need to print that much lines as user space
> will anyway notice that the memory cgroup has reached its limit.

But how do you debug why it is reaching the limit and why a particular
process has been killed?

If we are printing too much then OK, let's remove those parts which are
not that useful but hiding information which tells us more about the oom
decision doesn't sound right to me.

> Signed-off-by: Richard Weinberger <richard@nod.at>
[...]
-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC][PATCH] oom: Be less verbose if the oom_control event fd has listeners

[Richard Weinberger]

Am 05.06.2014 17:00, schrieb Michal Hocko:
> On Thu 05-06-14 16:00:41, Richard Weinberger wrote:
>> Don't spam the kernel logs if the oom_control event fd has listeners.
>> In this case there is no need to print that much lines as user space
>> will anyway notice that the memory cgroup has reached its limit.
> 
> But how do you debug why it is reaching the limit and why a particular
> process has been killed?

In my case it's always because customer's Java application gone nuts.
So I don't really have to debug a lot. ;-)
But I can understand your point.

> If we are printing too much then OK, let's remove those parts which are
> not that useful but hiding information which tells us more about the oom
> decision doesn't sound right to me.

What about adding a sysctl like "vm.oom_verbose"?
By default it would be 1.
If set to 0 the full OOM information is only printed out if nobody listens
to the event fd.

Thanks,
//richard

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC][PATCH] oom: Be less verbose if the oom_control event fd has listeners

[Michal Hocko]

On Thu 05-06-14 17:55:54, Richard Weinberger wrote:
> Am 05.06.2014 17:00, schrieb Michal Hocko:
> > On Thu 05-06-14 16:00:41, Richard Weinberger wrote:
> >> Don't spam the kernel logs if the oom_control event fd has listeners.
> >> In this case there is no need to print that much lines as user space
> >> will anyway notice that the memory cgroup has reached its limit.
> > 
> > But how do you debug why it is reaching the limit and why a particular
> > process has been killed?
> 
> In my case it's always because customer's Java application gone nuts.
> So I don't really have to debug a lot. ;-)
> But I can understand your point.

If you know that handling memcg-OOM condition is easy then maybe you can
not only listen for the OOM notifications but also handle OOM conditions
and kill the offender. This would mean that kernel doesn't try to kill
anything and so wouldn't dump anything to the log.
 
> > If we are printing too much then OK, let's remove those parts which are
> > not that useful but hiding information which tells us more about the oom
> > decision doesn't sound right to me.
> 
> What about adding a sysctl like "vm.oom_verbose"?
> By default it would be 1.
> If set to 0 the full OOM information is only printed out if nobody listens
> to the event fd.

If we have a knob then I guess it should be global and shared by memcg
as well. I can imagine that somebody might be interested only in the
tasks dump, while somebody would like to see LRU states and other memory
counters. So it would be ideally a bitmask of things to output. I do not
think that a memcg specific solution is good, though.

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC][PATCH] oom: Be less verbose if the oom_control event fd has listeners

[Richard Weinberger]

Am 05.06.2014 18:18, schrieb Michal Hocko:
> On Thu 05-06-14 17:55:54, Richard Weinberger wrote:
>> Am 05.06.2014 17:00, schrieb Michal Hocko:
>>> On Thu 05-06-14 16:00:41, Richard Weinberger wrote:
>>>> Don't spam the kernel logs if the oom_control event fd has listeners.
>>>> In this case there is no need to print that much lines as user space
>>>> will anyway notice that the memory cgroup has reached its limit.
>>>
>>> But how do you debug why it is reaching the limit and why a particular
>>> process has been killed?
>>
>> In my case it's always because customer's Java application gone nuts.
>> So I don't really have to debug a lot. ;-)
>> But I can understand your point.
> 
> If you know that handling memcg-OOM condition is easy then maybe you can
> not only listen for the OOM notifications but also handle OOM conditions
> and kill the offender. This would mean that kernel doesn't try to kill
> anything and so wouldn't dump anything to the log.

Basically I don't care what customers run in their containers.
But almost every OOM is because their Java apps consume too much memory.
Mostly because they don't know exactly how much memory they need or
because of completely broken JVM heap settings.

All my OOM listener does is sending a mail a la "Your container ran out of memory, go figure...".

>>> If we are printing too much then OK, let's remove those parts which are
>>> not that useful but hiding information which tells us more about the oom
>>> decision doesn't sound right to me.
>>
>> What about adding a sysctl like "vm.oom_verbose"?
>> By default it would be 1.
>> If set to 0 the full OOM information is only printed out if nobody listens
>> to the event fd.
> 
> If we have a knob then I guess it should be global and shared by memcg
> as well. I can imagine that somebody might be interested only in the
> tasks dump, while somebody would like to see LRU states and other memory
> counters. So it would be ideally a bitmask of things to output. I do not
> think that a memcg specific solution is good, though.

I'm not sure if such a fine grained setting is really useful.

Thanks,
//richard

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC][PATCH] oom: Be less verbose if the oom_control event fd has listeners

[David Rientjes]

On Thu, 5 Jun 2014, Michal Hocko wrote:

> If we are printing too much then OK, let's remove those parts which are
> not that useful but hiding information which tells us more about the oom
> decision doesn't sound right to me.
> 

Memcg oom killer printing is controlled mostly by 
mem_cgroup_print_oom_info(), I don't see anything in the generic oom 
killer that should be removed and that I have not used even for memcg ooms 
in the past.

Perhaps there could be a case made for suppressing some of the 
hierarchical stats from being printed for memcg ooms and controlled by 
another memcg knob, but it doesn't sound vital.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC][PATCH] oom: Be less verbose if the oom_control event fd has listeners

[Michal Hocko]

On Thu 05-06-14 14:01:02, David Rientjes wrote:
> On Thu, 5 Jun 2014, Michal Hocko wrote:
> 
> > If we are printing too much then OK, let's remove those parts which are
> > not that useful but hiding information which tells us more about the oom
> > decision doesn't sound right to me.
> > 
> 
> Memcg oom killer printing is controlled mostly by 
> mem_cgroup_print_oom_info(), I don't see anything in the generic oom 
> killer that should be removed and that I have not used even for memcg ooms 
> in the past.

Yes, I find most of the information printed during OOM very helpful.
After 58cf188ed649 (memcg, oom: provide more precise dump info while
memcg oom happening) even memcg oom info is helpful.

> Perhaps there could be a case made for suppressing some of the 
> hierarchical stats from being printed for memcg ooms and controlled by 
> another memcg knob, but it doesn't sound vital.

Agreed.

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>