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

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

> 
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 4b0a5d8b92e1..25dd7cd6fb5e 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -456,9 +456,12 @@ static bool __oom_reap_vmas(struct mm_struct *mm)
>  		 * we do not want to block exit_mmap by keeping mm ref
>  		 * count elevated without a good reason.
>  		 */
> -		if (vma_is_anonymous(vma) || !(vma->vm_flags & VM_SHARED))
> +		if (vma_is_anonymous(vma) || !(vma->vm_flags & VM_SHARED)) {
> +			if (vma->vm_flags & VM_LOCKED)
> +				munlock_vma_pages_all(vma);
>  			unmap_page_range(&tlb, vma, vma->vm_start, vma->vm_end,
>  					 &details);
> +		}
>  	}
>  	tlb_finish_mmu(&tlb, 0, -1);
>  	up_read(&mm->mmap_sem);
-- 
 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>