On 05/24/2017 02:10 PM, Xishi Qiu wrote:
> On 2017/5/24 19:52, Vlastimil Babka wrote:
>
>> On 05/24/2017 01:38 PM, Xishi Qiu wrote:
>>>>
>>>> Race condition with what? Who else would isolate our pages?
>>>>
>>>
>>> Hi Vlastimil,
>>>
>>> I find the root cause, if the page was not cached on the current cpu,
>>> lru_add_drain() will not push it to LRU. So we should handle fail
>>> case in mlock_vma_page().
>>
>> Yeah that would explain it.
>>
>>> follow_page_pte()
>>> ...
>>> if (page->mapping && trylock_page(page)) {
>>> lru_add_drain(); /* push cached pages to LRU */
>>> /*
>>> * Because we lock page here, and migration is
>>> * blocked by the pte's page reference, and we
>>> * know the page is still mapped, we don't even
>>> * need to check for file-cache page truncation.
>>> */
>>> mlock_vma_page(page);
>>> unlock_page(page);
>>> }
>>> ...
>>>
>>> I think we should add yisheng's patch, also we should add the following change.
>>> I think it is better than use lru_add_drain_all().
>>
>> 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.
> 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 majordomo@xxxxxxxxx. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
