Re: can't oom-kill zap the victim's memory?

From: David Rientjes
Date: Tue Sep 22 2015 - 19:04:43 EST


On Tue, 22 Sep 2015, Oleg Nesterov wrote:

> Finally. Whatever we do, we need to change oom_kill_process() first,
> and I think we should do this regardless. The "Kill all user processes
> sharing victim->mm" logic looks wrong and suboptimal/overcomplicated.
> I'll try to make some patches tomorrow if I have time...
>

Killing all processes sharing the ->mm has been done in the past to
obviously ensure that memory is eventually freed, but also to solve
mm->mmap_sem livelocks where a thread is holding a contended mutex and
needs a fatal signal to acquire TIF_MEMDIE if it calls into the oom killer
and be able to allocate so that it may eventually drop the mutex.

> But. Can't we just remove another ->oom_score_adj check when we try
> to kill all mm users (the last for_each_process loop). If yes, this
> all can be simplified.
>

For complete correctness, we would avoid killing any process that shares
memory with an oom disabled thread since the oom killer shall not kill it
and otherwise we do not free any memory.

> I guess we can't and its a pity. Because it looks simply pointless
> to not kill all mm users. This just means the select_bad_process()
> picked the wrong task.
>

This is a side-effect of moving oom scoring to signal_struct from
mm_struct. It could be improved separately by flagging mm_structs that
are unkillable which would also allow for an optimization in
find_lock_task_mm().

> And while this completely offtopic... why does it take task_lock()
> to protect ->comm? Sure, without task_lock() we can print garbage.
> Is it really that important? I am asking because sometime people
> think that it is not safe to use ->comm lockless, but this is not
> true.
>

This has come up a couple times in the past and, from what I recall,
Andrew has said that we don't actually care since the string will always
be terminated and if we race we don't actually care. There are other
places in the kernel where task_lock() isn't used solely to protect
->comm. It can be removed from the oom_kill_process() loop checking for
other potential victims.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/