On 2017/5/24 21:16, Vlastimil Babka wrote:
> 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.
>
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.
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 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>
