Re: [PATCH] mm, oom: Introduce time limit for dump_tasks duration.

[Michal Hocko <mhocko@kernel.org>]

On Thu 06-09-18 22:45:26, Tetsuo Handa wrote:
> On 2018/09/06 20:53, Michal Hocko wrote:
> > On Thu 06-09-18 20:40:34, Tetsuo Handa wrote:
> >> On 2018/09/06 20:23, Michal Hocko wrote:
> >>> On Thu 06-09-18 19:58:25, Tetsuo Handa wrote:
> >>> [...]
> >>>> >From 18876f287dd69a7c33f65c91cfcda3564233f55e Mon Sep 17 00:00:00 2001
> >>>> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> >>>> Date: Thu, 6 Sep 2018 19:53:18 +0900
> >>>> Subject: [PATCH] mm, oom: Introduce time limit for dump_tasks duration.
> >>>>
> >>>> Since printk() is slow, printing one line takes nearly 0.01 second.
> >>>> As a result, syzbot is stalling for 52 seconds trying to dump 5600
> >>>> tasks at for_each_process() under RCU. Since such situation is almost
> >>>> inflight fork bomb attack (the OOM killer will print similar tasks for
> >>>> so many times), it makes little sense to print all candidate tasks.
> >>>> Thus, this patch introduces 3 seconds limit for printing.
> >>>>
> >>>> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> >>>> Cc: Dmitry Vyukov <dvyukov@google.com>
> >>>
> >>> You really love timeout based solutions with randomly chosen timeouts,
> >>> don't you. This is just ugly as hell. We already have means to disable
> >>> tasks dumping (see /proc/sys/vm/oom_dump_tasks).
> >>
> >> I know /proc/sys/vm/oom_dump_tasks . Showing some entries while not always
> >> printing all entries might be helpful.
> > 
> > Not really. It could be more confusing than helpful. The main purpose of
> > the listing is to double check the list to understand the oom victim
> > selection. If you have a partial list you simply cannot do that.
> 
> It serves as a safeguard for avoiding RCU stall warnings.
> 
> > 
> > If the iteration takes too long and I can imagine it does with zillions
> > of tasks then the proper way around it is either release the lock
> > periodically after N tasks is processed or outright skip the whole thing
> > if there are too many tasks. The first option is obviously tricky to
> > prevent from duplicate entries or other artifacts.
> > 
> 
> Can we add rcu_lock_break() like check_hung_uninterruptible_tasks() does?

This would be a better variant of your timeout based approach. But it
can still produce an incomplete task list so it still consumes a lot of
resources to print a long list of tasks potentially while that list is not
useful for any evaluation. Maybe that is good enough. I don't know. I
would generally recommend to disable the whole thing with workloads with
many tasks though.

-- 
Michal Hocko
SUSE Labs