Re: [RFC PATCH] mm, oom: oom ratelimit auto tuning

[Yafang Shao <laoar.shao@gmail.com>]

On Tue, Apr 14, 2020 at 3:39 PM Michal Hocko <mhocko@kernel.org> wrote:
>
> On Sat 11-04-20 05:36:14, Yafang Shao wrote:
> > Recently we find an issue that when OOM happens the server is almost
> > unresponsive for several minutes. That is caused by a slow serial set
> > with "console=ttyS1,19200". As the speed of this serial is too slow, it
> > will take almost 10 seconds to print a full OOM message into it. And
> > then all tasks allocating pages will be blocked as there is almost no
> > pages can be reclaimed. At that time, the memory pressure is around 90
> > for a long time. If we don't print the OOM messages into this serial,
> > a full OOM message only takes less than 1ms and the memory pressure is
> > less than 40.
>
> Which part of the oom report takes the most time? I would expect this to
> be the dump_tasks part which can be pretty large when there is a lot of
> eligible tasks to kill.
>

Yes, dump_tasks takes around 6s of the total 10s,  show_mem take
around 2s, and dump_stack takes around 0.8s.

> > We can avoid printing OOM messages into slow serial by adjusting
> > /proc/sys/kernel/printk to fix this issue, but then all messages with
> > KERN_WARNING level can't be printed into it neither, that may loss some
> > useful messages when we want to collect messages from the it for
> > debugging purpose.
>
> A large part of the oom report is printed with KERN_INFO log level. So
> you can reduce a large part of the output while not losing other
> potentially important information.
>

Reduce the KERN_INFO log can save lots of time, but I just worried
that sometimes the user may need the full log and if then can't find
these logs they may complain.

> > So it is better to decrease the ratelimit. We can introduce some sysctl
> > knobes similar with printk_ratelimit and burst, but it will burden the
> > amdin. Let the kernel automatically adjust the ratelimit, that would be
> > a better choice.
>
> No new knobs for ratelimiting. Admin shouldn't really care about these
> things.

Agreed.

[snip]
> Besides that I strongly suspect that you would be much better of
> by disabling /proc/sys/vm/oom_dump_tasks which would reduce the amount
> of output a lot. Or do you really require this information when
> debugging oom reports?
>

Yes, disabling /proc/sys/vm/oom_dump_tasks can save lots of time.
But I'm not sure whehter we can disable it totally, because disabling
it would prevent the tasks log from being wrote into /var/log/messages
neither.

> > The OOM ratelimit starts with a slow rate, and it will increase slowly
> > if the speed of the console is rapid and decrease rapidly if the speed
> > of the console is slow. oom_rs.burst will be in [1, 10] and
> > oom_rs.interval will always greater than 5 * HZ.
>
> I am not against increasing the ratelimit timeout. But this patch seems
> to be trying to be too clever.  Why cannot we simply increase the
> parameters of the ratelimit?

I justed worried that the user may complain it if too many
oom_kill_process callbacks are suppressed.
But considering that OOM burst at the same time are always because of
the same reason, so I think one snapshot of the OOM may be enough.
Simply setting oom_rs with {20 * HZ, 1} can resolve this issue.

> I am also interested whether this actually
> works. AFAIR ratelimit doesn't really work reliably when the ratelimited
> operation takes a long time because the internals have no way to see
> when the operation finished.
>

Agree with you that ratelimit() was not so reliable.

> >  mm/oom_kill.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++---
> >  1 file changed, 48 insertions(+), 3 deletions(-)
> >
> > diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> > index dfc357614e56..23dba8ccf313 100644
> > --- a/mm/oom_kill.c
> > +++ b/mm/oom_kill.c
> > @@ -954,8 +954,10 @@ static void oom_kill_process(struct oom_control *oc, const char *message)
> >  {
> >       struct task_struct *victim = oc->chosen;
> >       struct mem_cgroup *oom_group;
> > -     static DEFINE_RATELIMIT_STATE(oom_rs, DEFAULT_RATELIMIT_INTERVAL,
> > -                                           DEFAULT_RATELIMIT_BURST);
> > +     static DEFINE_RATELIMIT_STATE(oom_rs, 20 * HZ, 1);
> > +     int delta;
> > +     unsigned long start;
> > +     unsigned long end;
> >
> >       /*
> >        * If the task is already exiting, don't alarm the sysadmin or kill
> > @@ -972,8 +974,51 @@ static void oom_kill_process(struct oom_control *oc, const char *message)
> >       }
> >       task_unlock(victim);
> >
> > -     if (__ratelimit(&oom_rs))
> > +     if (__ratelimit(&oom_rs)) {
> > +             start = jiffies;
> >               dump_header(oc, victim);
> > +             end = jiffies;
> > +             delta = end - start;
> > +
> > +             /*
> > +              * The OOM messages may be printed to a serial with very low
> > +              * speed, e.g. console=ttyS1,19200. It will take long
> > +              * time to print these OOM messages to this serial, and
> > +              * then processes allocating pages will all be blocked due
> > +              * to it can hardly reclaim pages. That will case high
> > +              * memory pressure and the system may be unresponsive for a
> > +              * long time.
> > +              * In this case, we should decrease the OOM ratelimit or
> > +              * avoid printing OOM messages into the slow serial. But if
> > +              * we avoid printing OOM messages into the slow serial, all
> > +              * messages with KERN_WARNING level can't be printed into
> > +              * it neither, that may loss some useful messages when we
> > +              * want to collect messages from the console for debugging
> > +              * purpose. So it is better to decrease the ratelimit. We
> > +              * can introduce some sysctl knobes similar with
> > +              * printk_ratelimit and burst, but it will burden the
> > +              * admin. Let the kernel automatically adjust the ratelimit
> > +              * would be a better chioce.
> > +              * In bellow algorithm, it will decrease the OOM ratelimit
> > +              * rapidly if the console is slow and increase the OOM
> > +              * ratelimit slowly if the console is fast. oom_rs.burst
> > +              * will be in [1, 10] and oom_rs.interval will always
> > +              * greater than 5 * HZ.
> > +              */
> > +             if (delta < oom_rs.interval / 10) {
> > +                     if (oom_rs.interval >= 10 * HZ)
> > +                             oom_rs.interval /= 2;
> > +                     else if (oom_rs.interval > 6 * HZ)
> > +                             oom_rs.interval -= HZ;
> > +
> > +                     if (oom_rs.burst < 10)
> > +                             oom_rs.burst += 1;
> > +             } else if (oom_rs.burst > 1) {
> > +                     oom_rs.burst = 1;
> > +                     oom_rs.interval = 4 * delta;
> > +             }
> > +
> > +     }
> >
> >       /*
> >        * Do we need to kill the entire memory cgroup?
> > --
> > 2.18.2
>
> --


Thanks
Yafang