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

[Michal Hocko <mhocko@suse.cz>]

On Tue 05-01-16 17:03:12, Kirill A. Shutemov wrote:
> 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.

Sorry, I meant your fixup should be folded. The one to allow munlock as
a separate fix.
> 
> > > 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?

The target process is the OOM victim. It can be doing anything.
Including holding the page lock. But it cannot be holding page lock
while doing the allocation or invoking the OOM killer because that would
be a deadlock already even without oom reaper in the picture.

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