> On Oct 27, 2015, at 16:52, yalin wang <yalin.wang2...@gmail.com> wrote: > > >> On Oct 27, 2015, at 16:10, Minchan Kim <minc...@kernel.org> wrote: >> >> On Tue, Oct 27, 2015 at 03:39:16PM +0800, yalin wang wrote: >>> >>>> On Oct 27, 2015, at 15:09, Minchan Kim <minc...@kernel.org> wrote: >>>> >>>> Hello Yalin, >>>> >>>> Sorry for missing you in Cc list. >>>> IIRC, mails to send your previous mail address(yalin.w...@sonymobile.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.wang2...@gmail.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 oh, i have see 0b502297d1cc26e09b98955b4efa728be1c48921 this commit merged , then this problem should be fixed by this method. ignore this mail. :)
Thanks a lot . -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
