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

[Bob Liu <bob.liu@oracle.com>]

On 12/18/2013 10:02 AM, Wanpeng Li wrote:
> 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. 

If mlock_vma_page() really need pagelocked, then we have to add
lock_page() in try_to_unmap_cluster().
Else just remove the BUG_ON() to make things simple.

-- 
Regards,
-Bob

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