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!

> 
--
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/

Reply via email to