RE: mm: kernel BUG at mm/mlock.c:82!

[Motohiro Kosaki <Motohiro.Kosaki@us.fujitsu.com>]

> -----Original Message-----
> From: Wanpeng Li [mailto:liwanp@linux.vnet.ibm.com]
> Sent: Tuesday, December 17, 2013 9:03 PM
> To: Motohiro Kosaki
> Cc: Bob Liu; Andrew Morton; linux-mm@kvack.org; Michel Lespinasse;
> npiggin@suse.de; Motohiro Kosaki JP; riel@redhat.com; Sasha Levin
> Subject: Re: mm: kernel BUG at mm/mlock.c:82!
> 
> Hi Motohiro,
> On Tue, Dec 17, 2013 at 08:32:49AM -0800, Motohiro Kosaki wrote:
> >
> >
> >> -----Original Message-----
> >> From: owner-linux-mm@kvack.org [mailto:owner-linux-mm@kvack.org]
> On
> >> Behalf Of Wanpeng Li
> >> Sent: Tuesday, December 17, 2013 3:27 AM
> >> To: Sasha Levin
> >> Cc: Bob Liu; Andrew Morton; linux-mm@kvack.org; Michel Lespinasse;
> >> npiggin@suse.de; Motohiro Kosaki JP; riel@redhat.com
> >> Subject: Re: mm: kernel BUG at mm/mlock.c:82!
> >>
> >> Hi Sasha,
> >> On Tue, Dec 17, 2013 at 01:46:54AM -0500, Sasha Levin wrote:
> >> >On 12/17/2013 12:39 AM, Bob Liu wrote:
> >> >>cc'd more people.
> >> >>
> >> >>On 12/17/2013 09:04 AM, Sasha Levin wrote:
> >> >>>Hi all,
> >> >>>
> >> >>>While fuzzing with trinity inside a KVM tools guest running latest
> >> >>>-next kernel, I've stumbled on the following spew.
> >> >>>
> >> >>>Codewise, it's pretty straightforward. In try_to_unmap_cluster():
> >> >>>
> >> >>>                 page = vm_normal_page(vma, address, *pte);
> >> >>>                 BUG_ON(!page || PageAnon(page));
> >> >>>
> >> >>>                 if (locked_vma) {
> >> >>>                         mlock_vma_page(page);   /* no-op if already
> >> >>>mlocked */
> >> >>>                         if (page == check_page)
> >> >>>                                 ret = SWAP_MLOCK;
> >> >>>                         continue;       /* don't unmap */
> >> >>>                 }
> >> >>>
> >> >>>And the BUG triggers once we see that 'page' isn't locked.
> >> >>>
> >> >>
> >> >>Yes, I didn't see any place locked the corresponding page in
> >> >>try_to_unmap_cluster().
> >> >>
> >> >>I'm afraid adding lock_page() over there may cause potential deadlock.
> >> >>How about just remove the BUG_ON() in mlock_vma_page()?
> >> >
> >> >Welp, it's been there for 5 years now - there should be a good
> >> >reason to
> >> justify removing it.
> >> >
> >>
> >> Page should be locked before invoke try_to_unmap(), this check can't
> >> be removed since this bug is just triggered by confirm !check page
> >> hold page lock in virtual scan during nolinear VMAs pages aging.
> >> Avoid to confirm !check page hold page lock is acceptable.
> >
> >That's a try_to_unmap()'s assumption and it already have
> BUG_ON(!PageLocked(page)).
> >We can remove wrong BUG_ON from mlock_vma_page() simply.
> Mlock_vma_page() doesn't depend on page-locked.
> >
> 
> There is a race between mlock_vma_page() and munlock_vma_page(). Both
> of them should hold page lock and have a BUG_ON assumption.

Please explain which race you are worried.  The main race of mlock and munlock are closed by PG_mlocked, not PG_locked.


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