Re: [PATCH] mm: try to free swap only for reading swap fault

[Michal Hocko]

On Thu 02-11-17 20:35:19, zhouxianrong@xxxxxxxxxx wrote:
> From: zhouxianrong <zhouxianrong@xxxxxxxxxx>
> 
> the purpose of this patch is that when a reading swap fault
> happens on a clean swap cache page whose swap count is equal
> to one, then try_to_free_swap could remove this page from 
> swap cache and mark this page dirty. so if later we reclaimed
> this page then we could pageout this page due to this dirty.
> so i want to allow this action only for writing swap fault.
> 
> i sampled the data of non-dirty anonymous pages which is no
> need to pageout and total anonymous pages in shrink_page_list.
> 
> the results are:
> 
>         non-dirty anonymous pages     total anonymous pages
> before  26343                         635218
> after   36907                         634312

This data is absolutely pointless without describing the workload.
You patch also stil fails to explain which workloads are going to
benefit/suffer from the change and why it is a good thing to do in
general.

> Signed-off-by: zhouxianrong <zhouxianrong@xxxxxxxxxx>
> ---
>  mm/memory.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index a728bed..5a944fe 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2999,7 +2999,7 @@ int do_swap_page(struct vm_fault *vmf)
>  	}
>  
>  	swap_free(entry);
> -	if (mem_cgroup_swap_full(page) ||
> +	if (((vmf->flags & FAULT_FLAG_WRITE) && mem_cgroup_swap_full(page)) ||
>  	    (vma->vm_flags & VM_LOCKED) || PageMlocked(page))
>  		try_to_free_swap(page);
>  	unlock_page(page);
> -- 
> 1.7.9.5
> 

-- 
Michal Hocko
SUSE Labs