> On Oct 27, 2015, at 16:10, Minchan Kim <minchan@xxxxxxxxxx> wrote: > > On Tue, Oct 27, 2015 at 03:39:16PM +0800, yalin wang wrote: >> >>> On Oct 27, 2015, at 15:09, Minchan Kim <minchan@xxxxxxxxxx> wrote: >>> >>> Hello Yalin, >>> >>> Sorry for missing you in Cc list. >>> IIRC, mails to send your previous mail address(Yalin.Wang@xxxxxxxxxxxxxx) >>> 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@xxxxxxxxx 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, 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