Re: [PATCH v3] mm: fix trying to reclaim unevictable lru page when calling madvise_pageout

[zhong jiang <zhongjiang@huawei.com>]

On 2019/11/6 2:48, Johannes Weiner wrote:
> On Tue, Nov 05, 2019 at 10:10:37PM +0800, zhong jiang wrote:
>> On 2019/11/5 20:45, Michal Hocko wrote:
>>> On Tue 05-11-19 20:28:58, zhong jiang wrote:
>>>> On 2019/11/5 14:33, Michal Hocko wrote:
>>>>> On Sat 02-11-19 15:36:55, zhong jiang wrote:
>>>>>> On 2019/11/2 2:32, Michal Hocko wrote:
>>>>> [...]
>>>>>>> But I would really appreciate to add a comment for the BUG_ON and
>>>>>>> explain why do we care about PageUnevictable so much when there is an
>>>>>>> explicit page_evictable check in the reclaim path. In other words a
>>>>>>> short summary of what Johannes explained in
>>>>>>> http://lkml.kernel.org/r/20191030193307.GA48128@cmpxchg.org. Maybe in a
>>>>>>> separate patch. Care to send one or should I send it?
>>>>>> Hi,  Michal
>>>>>>
>>>>>> Actually,  I am not very clear about the words Johannes had said.   How the race to
>>>>>> tirgger, it will result in an PgeMlocked page can be visible in shrink_page_list.
>>>>>>
>>>>>> Could you elaborate the race in detail further ?
>>>>> I would go with the following comment
>>>>>
>>>>> 	/*
>>>>> 	 * Page reclaim can see !page_evictable(), but it must not see pages that
>>>>> 	 * have the PageUnevictable lru bit already set. See  __pagevec_lru_add_fn()
>>>>> 	 * for more details.
>>>>> 	 */
>>>> But  the detail still confuses me in __pagevec_lru_add_fn()  to  see PageMlocked  in vmscan :-\ .
>>> Which part does confuse you exactly?
>> page reclaim can see !page_evictable()  means some race still exist in the kernel. Is there any race window .
> Yes. mlock does this:
>
> lock_page()
> SetPageMlocked()
> if (isolate_lru_page())
>   putback_lru_page() // move to unevictable list
> unlock_page()
>
> and vmscan does this:
>
> isolate_lru_pages()
> for_each_page()
>   if (!try_lock_page())
>     continue
>   if (!page_evictable())
>     continue
> putback_lru_pages()
>
> It's possible that mlock locks the page and sets PG_mlocked, but
> vmscan has the page already isolated and mlock cannot move it to the
> unevictable list itself. In that case, vmscan will either fail to lock
> the page or see !page_unevictable() and move the page on the
> unevictable list on behalf of mlock.
Thanks for you kindly clarification.  

Sincerely,
zhong jiang
> .
>