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

["Kirill A. Shutemov" <kirill@shutemov.name>]

On Tue, Jan 05, 2016 at 02:31:23PM +0100, Michal Hocko wrote:
> 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.

I prefer it not to be folded. To be able to revert in something go wrong.

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

Can target process hold page lock? Or a process in direct replaim?
Basically, I don't know what I'm talking about ;-P

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

I haven't subscribed to implementing this just yet :)

-- 
 Kirill A. Shutemov

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