Re: [PATCH] mm, oom: avoid printk() iteration under RCU

[Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>]

On 2019/07/18 23:02, Michal Hocko wrote:
> On Thu 18-07-19 22:50:14, Tetsuo Handa wrote:
>> On 2019/07/18 17:30, Michal Hocko wrote:
>>> On Wed 17-07-19 19:55:01, Tetsuo Handa wrote:
>>>> Currently dump_tasks() might call printk() for many thousands times under
>>>> RCU, which might take many minutes for slow consoles.
>>>
>>> Is is even wise to enable dumping tasks on systems with thousands of
>>> tasks and slow consoles? I mean you still have to call printk that is
>>> slow that many times. So why do we actually care? Because of RCU stall
>>> warnings?
>>>
>>
>> That's a stupid question. WE DO CARE.
> 
> -ENOARGUMENT
> 
>> We are making efforts for avoid calling printk() on each thread group (e.g.
>>
>>   commit 0c1b2d783cf34324 ("mm/oom_kill: remove the wrong fatal_signal_pending() check in oom_kill_process()")
> 
> removes fatal_signal_pending rather than focusing on printk

No. Its focus is to suppress printk(), for it fixes fatal_signal_pending() test
introduced by commit 840807a8f40bb25a ("mm/oom_kill.c: suppress unnecessary
"sharing same memory" message").

> 
>>   commit b2b469939e934587 ("proc, oom: do not report alien mms when setting oom_score_adj"))
> 
> removes a printk of a dubious value.

No. Its focus is to remove printk(), for that printk() allows the system to
TASK_UNINTERRUPTIBLE stall for 44 days (even without slow consoles) in addition
to RCU stall for 2 minutes.

> 
>> ) under RCU and this patch is one of them (except that we can't remove
>> printk() for dump_tasks() case).
> 
> No, this one adds a complexity for something that is not clearly a huge
> win or the win is not explained properly.
> 

The win is already explained properly by the past commits. Avoiding RCU stalls
(even without slow consoles) is a clear win. The duration of RCU stall avoided
by this patch is roughly the same with commit b2b469939e934587.

We haven't succeeded making printk() asynchronous (and potentially we won't
succeed making printk() asynchronous because we need synchronous printk()
when something critical is undergoing outside of out_of_memory()). Thus,
bringing printk() to outside of RCU section is a clear win we can make for now.