Re: [PATCH V3 1/2] mm: avoid marking swap cached page as lazyfree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue 26-09-17 10:26:25, Shaohua Li wrote:
> From: Shaohua Li <shli@xxxxxx>
> 
> MADV_FREE clears pte dirty bit and then marks the page lazyfree (clear
> SwapBacked). There is no lock to prevent the page is added to swap cache
> between these two steps by page reclaim. Page reclaim could add the page
> to swap cache and unmap the page. After page reclaim, the page is added
> back to lru. At that time, we probably start draining per-cpu pagevec
> and mark the page lazyfree. So the page could be in a state with
> SwapBacked cleared and PG_swapcache set. Next time there is a refault in
> the virtual address, do_swap_page can find the page from swap cache but
> the page has PageSwapCache false because SwapBacked isn't set, so
> do_swap_page will bail out and do nothing. The task will keep running
> into fault handler.

Thanks for the clarification in the changelog. It is much more clear
now!

> Reported-and-tested-by: Artem Savkov <asavkov@xxxxxxxxxx>
> Fix: 802a3a92ad7a(mm: reclaim MADV_FREE pages)
> Signed-off-by: Shaohua Li <shli@xxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Cc: Johannes Weiner <hannes@xxxxxxxxxxx>
> Cc: Michal Hocko <mhocko@xxxxxxxx>
> Cc: Hillf Danton <hillf.zj@xxxxxxxxxxxxxxx>
> Cc: Minchan Kim <minchan@xxxxxxxxxx>
> Cc: Hugh Dickins <hughd@xxxxxxxxxx>
> Cc: Mel Gorman <mgorman@xxxxxxxxxxxxxxxxxxx>
> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> Reviewed-by: Rik van Riel <riel@xxxxxxxxxx>

Marking for stable as suggested by Johannes makes perfect sense to me.
Acked-by: Michal Hocko <mhocko@xxxxxxxx>

> ---
>  mm/swap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/swap.c b/mm/swap.c
> index 9295ae9..a77d68f 100644
> --- a/mm/swap.c
> +++ b/mm/swap.c
> @@ -575,7 +575,7 @@ static void lru_lazyfree_fn(struct page *page, struct lruvec *lruvec,
>  			    void *arg)
>  {
>  	if (PageLRU(page) && PageAnon(page) && PageSwapBacked(page) &&
> -	    !PageUnevictable(page)) {
> +	    !PageSwapCache(page) && !PageUnevictable(page)) {
>  		bool active = PageActive(page);
>  
>  		del_page_from_lru_list(page, lruvec,
> @@ -665,7 +665,7 @@ void deactivate_file_page(struct page *page)
>  void mark_page_lazyfree(struct page *page)
>  {
>  	if (PageLRU(page) && PageAnon(page) && PageSwapBacked(page) &&
> -	    !PageUnevictable(page)) {
> +	    !PageSwapCache(page) && !PageUnevictable(page)) {
>  		struct pagevec *pvec = &get_cpu_var(lru_lazyfree_pvecs);
>  
>  		get_page(page);
> -- 
> 2.9.5
> 

-- 
Michal Hocko
SUSE Labs



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]