On 05/25/2017 03:16 AM, Xishi Qiu wrote:
> On 2017/5/24 21:16, Vlastimil Babka wrote:
>>>>
>>>> I agree about yisheng's fix (but v2 didn't address my comments). I don't
>>>> think we should add the hunk below, as that deviates from the rest of
>>>> the design.
>>>
>>> Hi Vlastimil,
>>>
>>> The rest of the design is that mlock should always success here, right?
>>
>> The rest of the design allows a temporary disconnect between mlocked
>> flag and being placed on unevictable lru.
>>
>>> If we don't handle the fail case, the page will be in anon/file lru list
>>> later when call __pagevec_lru_add(), but NR_MLOCK increased,
>>> this is wrong, right?
>>
>> It's not wrong, the page cannot get evicted even if on wrong lru, so
>> effectively it's already mlocked. We would be underaccounting NR_MLOCK.
>>
> 
> Hi Vlastimil,
> 
> I'm not quite understand why the page cannot get evicted even if on wrong lru.
> __isolate_lru_page() will only skip PageUnevictable(page), but this flag has 
> not
> been set, we only set PageMlocked.

The isolated page has to be unmapped from all vma's that map it. See
try_to_unmap_one() and this check:

                if (!(flags & TTU_IGNORE_MLOCK)) {
                        if (vma->vm_flags & VM_LOCKED) {
                                ...
                                ret = false;

This VM_LOCKED is what actually controls if page is evictable. The rest
is optimization (separate lru list so we don't scan the pages in reclaim
if they can't be evicted anyway), and accounting (PageMlocked flag pages
counted as NR_MLOCK). That's why temporary inconsistency isn't a problem.


> Thanks,
> Xishi Qiu
> 
>>> Thanks,
>>> Xishi Qiu
>>>
>>>>
>>>> Thanks,
>>>> Vlastimil
>>>>
>>>>> diff --git a/mm/mlock.c b/mm/mlock.c
>>>>> index 3d3ee6c..ca2aeb9 100644
>>>>> --- a/mm/mlock.c
>>>>> +++ b/mm/mlock.c
>>>>> @@ -88,6 +88,11 @@ void mlock_vma_page(struct page *page)
>>>>>           count_vm_event(UNEVICTABLE_PGMLOCKED);
>>>>>           if (!isolate_lru_page(page))
>>>>>                   putback_lru_page(page);
>>>>> +         else {
>>>>> +                 ClearPageMlocked(page);
>>>>> +                 mod_zone_page_state(page_zone(page), NR_MLOCK,
>>>>> +                                 -hpage_nr_pages(page));
>>>>> +         }
>>>>>   }
>>>>>  }
>>>>>
>>>>> Thanks,
>>>>> Xishi Qiu
>>>>>
>>>>
>>>>
>>>> .
>>>>
>>>
>>>
>>>
>>> --
>>> To unsubscribe, send a message with 'unsubscribe linux-mm' in
>>> the body to majord...@kvack.org.  For more info on Linux MM,
>>> see: http://www.linux-mm.org/ .
>>> Don't email: <a href=mailto:"d...@kvack.org";> em...@kvack.org </a>
>>>
>>
>>
>> .
>>
> 
> 
> 

Reply via email to