Re: [patch v2] mm, oom: fix concurrent munlock and oom reaper unmap

[Tetsuo Handa]

Michal Hocko wrote:
> > > Can we try a simpler way and get back to what I was suggesting before
> > > [1] and simply not play tricks with
> > > 		down_write(&mm->mmap_sem);
> > > 		up_write(&mm->mmap_sem);
> > > 
> > > and use the write lock in exit_mmap for oom_victims?
> > 
> > You mean something like this?
> 
> or simply hold the write lock until we unmap and free page tables.

That increases possibility of __oom_reap_task_mm() giving up reclaim and
setting MMF_OOM_SKIP when exit_mmap() is making forward progress, doesn't it?
I think that it is better that __oom_reap_task_mm() does not give up when
exit_mmap() can make progress. In that aspect, the section protected by
mmap_sem held for write should be as short as possible.

> It would make the locking rules much more straightforward.
> What you are proposing is more focused on this particular fix and it
> would work as well but the subtle locking would still stay in place.

Yes, this change is focused on -stable patch.

> I am not sure we want the trickiness.

I don't like the trickiness too. I think we can even consider direct OOM
reaping suggested at https://patchwork.kernel.org/patch/10095661/ .

> 
> > Then, I'm tempted to call __oom_reap_task_mm() before holding mmap_sem for write.
> > It would be OK to call __oom_reap_task_mm() at the beginning of __mmput()...
> 
> I am not sure I understand.

To reduce possibility of __oom_reap_task_mm() giving up reclaim and
setting MMF_OOM_SKIP.