On Mon, Jul 20, 2020 at 3:16 PM Michal Hocko <mhocko@xxxxxxxxxx> wrote:
>
> On Sun 19-07-20 09:53:15, Yafang Shao wrote:
> > When the OOM killer finding a victim and trying to kill it, if the victim
> > is already exiting, the task mm will be NULL and no process will be killed.
> > But the dump_header() has been already executed, so it will be strange to
> > dump so many information without killing a process. We'd better show some
> > helpful information to indicate why this happens.
> >
> > Suggested-by: David Rientjes <rientjes@xxxxxxxxxx>
> > Signed-off-by: Yafang Shao <laoar.shao@xxxxxxxxx>
> > Cc: Michal Hocko <mhocko@xxxxxxxxxx>
> > Cc: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx>
> > ---
> > mm/oom_kill.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> > index 6e94962..0480dde 100644
> > --- a/mm/oom_kill.c
> > +++ b/mm/oom_kill.c
> > @@ -863,9 +863,13 @@ static void __oom_kill_process(struct task_struct *victim, const char *message)
> >
> > p = find_lock_task_mm(victim);
> > if (!p) {
> > + pr_info("Process %d (%s) is already exiting\n",
> > + task_pid_nr(victim), victim->comm);
> > put_task_struct(victim);
>
> I do agree that a silent bail out is not the best thing to do. The above
> message would be more useful if it also explained what the oom killer
> does (or does not):
>
> "OOM victim %d (%s) is already exiting. Skip killing the task\n"
>
Sure.
> > return;
> > - } else if (victim != p) {
> > + }
> > +
> > + if (victim != p) {
>
> Why do we need this?
>
Because I don't like that code style.
But it is not a big problem, I will not change it in the next version.
> > get_task_struct(p);
> > put_task_struct(victim);
> > victim = p;
> > --
> > 1.8.3.1
>
> --
> Michal Hocko
> SUSE Labs
--
Thanks
Yafang
