[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <AE81166C-A1DD-4994-9FEA-B5E6BFAB1336@gmail.com>
Date: Tue, 27 Oct 2015 16:52:07 +0800
From: yalin wang <yalin.wang2010@...il.com>
To: Minchan Kim <minchan@...nel.org>
Cc: Hugh Dickins <hughd@...gle.com>,
Andrew Morton <akpm@...ux-foundation.org>,
"open list:MEMORY MANAGEMENT" <linux-mm@...ck.org>,
lkml <linux-kernel@...r.kernel.org>,
Rik van Riel <riel@...hat.com>, Mel Gorman <mgorman@...e.de>,
Michal Hocko <mhocko@...e.cz>,
Johannes Weiner <hannes@...xchg.org>,
"Kirill A. Shutemov" <kirill@...temov.name>,
Vlastimil Babka <vbabka@...e.cz>
Subject: Re: [PATCH 4/5] mm: simplify reclaim path for MADV_FREE
> On Oct 27, 2015, at 16:10, Minchan Kim <minchan@...nel.org> wrote:
>
> On Tue, Oct 27, 2015 at 03:39:16PM +0800, yalin wang wrote:
>>
>>> On Oct 27, 2015, at 15:09, Minchan Kim <minchan@...nel.org> wrote:
>>>
>>> Hello Yalin,
>>>
>>> Sorry for missing you in Cc list.
>>> IIRC, mails to send your previous mail address(Yalin.Wang@...ymobile.com)
>>> were returned.
>>>
>>> You added comment bottom line so I'm not sure what PageDirty you meant.
>>>
>>>> it is wrong here if you only check PageDirty() to decide if the page is freezable or not .
>>>> The Anon page are shared by multiple process, _mapcount > 1 ,
>>>> so you must check all pt_dirty bit during page_referenced() function,
>>>> see this mail thread:
>>>> http://ns1.ske-art.com/lists/kernel/msg1934021.html
>>>
>>> If one of pte among process sharing the page was dirty, the dirtiness should
>>> be propagated from pte to PG_dirty by try_to_unmap_one.
>>> IOW, if the page doesn't have PG_dirty flag, it means all of process did
>>> MADV_FREE.
>>>
>>> Am I missing something from you question?
>>> If so, could you show exact scenario I am missing?
>>>
>>> Thanks for the interest.
>> oh, yeah , that is right , i miss that , pte_dirty will propagate to PG_dirty ,
>> so that is correct .
>> Generic to say this patch move set_page_dirty() from add_to_swap() to
>> try_to_unmap(), i think can change a little about this patch:
>>
>> @@ -1476,6 +1446,8 @@ static int try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
>> ret = SWAP_FAIL;
>> goto out_unmap;
>> }
>> + if (!PageDirty(page))
>> + SetPageDirty(page);
>> if (list_empty(&mm->mmlist)) {
>> spin_lock(&mmlist_lock);
>> if (list_empty(&mm->mmlist))
>>
>> i think this 2 lines can be removed ,
>> since pte_dirty have propagated to set_page_dirty() , we don’t need this line here ,
>> otherwise you will always dirty a AnonPage, even it is clean,
>> then we will page out this clean page to swap partition one more , this is not needed.
>> am i understanding correctly ?
>
> Your understanding is correct.
> I will fix it in next spin.
>
>>
>> By the way, please change my mail address to yalin.wang2010@...il.com in CC list .
>> Thanks a lot. :)
>
> Thanks for the review!
i have a look at the old mail list , i recall the scenario that multiple processes share a AnonPage
special case :
for example Process A have a AnonPage map like this:
! pte_dirty() && PageDirty()==1 (this is possible after read fault happened on swap entry, and try_to_free_swap() succeed.)
Process A do a fork() , New process is called B .
Then A syscall(MADV_FREE) on the page .
At this time, page table like this:
A ! pte_dirty() && PageDirty() == 0 && PageSwapCache() == 0
B ! pte_dirty() && PageDirty() == 0 && PageSwapCache() == 0
This means this page is freeable , and can be freed during page reclaim.
This is not fair for Process B . Since B don’t call syscall(MADV_FREE) ,
its page should not be discard . Will cause some strange behaviour if happened .
This is discussed by
http://www.serverphorums.com/read.php?12,1220840
but i don’t know why the patch is not merged .
Thanks
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists