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

[Oleg Nesterov]

On 09/19, Linus Torvalds wrote:
>
> On Sat, Sep 19, 2015 at 8:03 AM, Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
> > +
> > +static void oom_unmap_func(struct work_struct *work)
> > +{
> > +       struct mm_struct *mm = xchg(&oom_unmap_mm, NULL);
> > +
> > +       if (!atomic_inc_not_zero(&mm->mm_users))
> > +               return;
> > +
> > +       // If this is not safe we can do use_mm() + unuse_mm()
> > +       down_read(&mm->mmap_sem);
>
> I don't think this is safe.
>
> What makes you sure that we might not deadlock on the mmap_sem here?
> For all we know, the process that is going out of memory is in the
> middle of a mmap(), and already holds the mmap_sem for writing. No?

In this case the workqueue thread will block. But it can not block
forever. I mean if it can then the killed process will never exit
(exit_mm does down_read) and release its memory, so we lose anyway.

But let me repeat this patch is obviously not complete/etc,

> So at the very least that needs to be a trylock, I think.

And we want to avoid using workqueues when the caller can do this
directly. And in this case we certainly need trylock. But this needs
some refactoring: we do not want to do this under oom_lock, otoh it
makes sense to do this from mark_oom_victim() if current && killed,
and a lot more details.

The workqueue thread has other reasons for trylock, but probably not
in the initial version of this patch. And perhaps we should use a
dedicated kthread and do not use workqueues at all. And yes, a single
"mm_struct *oom_unmap_mm" is ugly, it should be the list of mm's to
unmap, but then at least we need MMF_MEMDIE.

> And I'm not
> sure zap_page_range() is ok with the mmap_sem only held for reading.
> Normally our rule is that you can *populate* the page tables
> concurrently, but you can't tear the down.

Well, according to madvise_need_mmap_write() MADV_DONTNEED does this
under down_read().

But yes, yes, this is probably not right anyway. Say, VM_LOCKED...
That is why I mentioned that perhaps this should only unmap the
anonymous pages. We can probably add zap_details->for_oom hint.



Another question if it is safe to abuse the foreign mm this way.
Well, zap_page_range_single() does this, so this is probably safe.
But we can do use_mm().

Oleg.

--
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/