Re: [PATCH 1/2] mm, oom: skip mlocked VMAs in __oom_reap_vmas()

[Michal Hocko <mhocko@suse.cz>]

On Tue 05-01-16 15:10:39, Kirill A. Shutemov wrote:
> On Tue, Jan 05, 2016 at 01:47:35PM +0100, Michal Hocko wrote:
> > On Tue 29-12-15 23:46:29, Kirill A. Shutemov wrote:
> > > As far as I can see we explicitly munlock pages everywhere before unmap
> > > them. The only case when we don't to that is OOM-reaper.
> > 
> > Very well spotted!
> > 
> > > I don't think we should bother with munlocking in this case, we can just
> > > skip the locked VMA.
> > 
> > Why cannot we simply munlock them here for the private mappings?
> 
> It's probably right think to do, but I wanted to fix the bug first.

Fair enough. It is surely simpler, although I think we should tear
private mappings down even when mlocked. I can cook up a separate patch
on top of yours which is obviously correct and can be folded into the
original one.

> And I wasn't ready to investigate context the reaper working in to check
> if it's safe to munlock there. For instance, munlock would take page lock
> and I'm not sure at the moment if it can or cannot lead to deadlock in
> some scenario. So I choose safer fix.

repear is a flat kernel thread context which doesn't sit on any locks
(except for mmap sem for read taken on the way) so I do not immediately
see any potential for the dead lock. If the original context which
wakes it up depend on the page lock to move on then we would be screwed
already because we can end up doing exit_mmap in that context already
and so end up doing munlock as well.

> If calling munlock is always safe where unmap happens, why not move inside
> unmap?

This would be less error prone for sure. I would rather see it as a
separate patch which explains why it is safe in all cases though.
-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>