Re: [PATCH 08/10] mm: deactivations shouldn't bias the LRU balance

2016-06-08 Thread Michal Hocko
On Mon 06-06-16 15:48:34, Johannes Weiner wrote:
> Operations like MADV_FREE, FADV_DONTNEED etc. currently move any
> affected active pages to the inactive list to accelerate their reclaim
> (good) but also steer page reclaim toward that LRU type, or away from
> the other (bad).
> 
> The reason why this is undesirable is that such operations are not
> part of the regular page aging cycle, and rather a fluke that doesn't
> say much about the remaining pages on that list. They might all be in
> heavy use. But once the chunk of easy victims has been purged, the VM
> continues to apply elevated pressure on the remaining hot pages. The
> other LRU, meanwhile, might have easily reclaimable pages, and there
> was never a need to steer away from it in the first place.
> 
> As the previous patch outlined, we should focus on recording actually
> observed cost to steer the balance rather than speculating about the
> potential value of one LRU list over the other. In that spirit, leave
> explicitely deactivated pages to the LRU algorithm to pick up, and let
> rotations decide which list is the easiest to reclaim.
> 
> Signed-off-by: Johannes Weiner 

Acked-by: Michal Hocko 

> ---
>  mm/swap.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/mm/swap.c b/mm/swap.c
> index 645d21242324..ae07b469ddca 100644
> --- a/mm/swap.c
> +++ b/mm/swap.c
> @@ -538,7 +538,6 @@ static void lru_deactivate_file_fn(struct page *page, 
> struct lruvec *lruvec,
>  
>   if (active)
>   __count_vm_event(PGDEACTIVATE);
> - lru_note_cost(lruvec, !file, hpage_nr_pages(page));
>  }
>  
>  
> @@ -546,7 +545,6 @@ static void lru_deactivate_fn(struct page *page, struct 
> lruvec *lruvec,
>   void *arg)
>  {
>   if (PageLRU(page) && PageActive(page) && !PageUnevictable(page)) {
> - int file = page_is_file_cache(page);
>   int lru = page_lru_base_type(page);
>  
>   del_page_from_lru_list(page, lruvec, lru + LRU_ACTIVE);
> @@ -555,7 +553,6 @@ static void lru_deactivate_fn(struct page *page, struct 
> lruvec *lruvec,
>   add_page_to_lru_list(page, lruvec, lru);
>  
>   __count_vm_event(PGDEACTIVATE);
> - lru_note_cost(lruvec, !file, hpage_nr_pages(page));
>   }
>  }
>  
> -- 
> 2.8.3

-- 
Michal Hocko
SUSE Labs


Re: [PATCH 08/10] mm: deactivations shouldn't bias the LRU balance

2016-06-08 Thread Minchan Kim
On Mon, Jun 06, 2016 at 03:48:34PM -0400, Johannes Weiner wrote:
> Operations like MADV_FREE, FADV_DONTNEED etc. currently move any
> affected active pages to the inactive list to accelerate their reclaim
> (good) but also steer page reclaim toward that LRU type, or away from
> the other (bad).
> 
> The reason why this is undesirable is that such operations are not
> part of the regular page aging cycle, and rather a fluke that doesn't
> say much about the remaining pages on that list. They might all be in
> heavy use. But once the chunk of easy victims has been purged, the VM
> continues to apply elevated pressure on the remaining hot pages. The
> other LRU, meanwhile, might have easily reclaimable pages, and there
> was never a need to steer away from it in the first place.
> 
> As the previous patch outlined, we should focus on recording actually
> observed cost to steer the balance rather than speculating about the
> potential value of one LRU list over the other. In that spirit, leave
> explicitely deactivated pages to the LRU algorithm to pick up, and let
> rotations decide which list is the easiest to reclaim.
> 
> Signed-off-by: Johannes Weiner 

Nice description. Agreed.

Acked-by: Minchan Kim