Re: + mm-thp-avoid-unnecessary-swapin-in-khugepaged.patch added to -mm tree

From: "Kirill A. Shutemov" <kirill@shutemov.name>
To: Michal Hocko <mhocko@kernel.org>
Cc: akpm@linux-foundation.org, ebru.akagunduz@gmail.com,
	aarcange@redhat.com, aneesh.kumar@linux.vnet.ibm.com,
	boaz@plexistor.com, gorcunov@openvz.org, hannes@cmpxchg.org,
	hughd@google.com, iamjoonsoo.kim@lge.com,
	kirill.shutemov@linux.intel.com, mgorman@suse.de,
	n-horiguchi@ah.jp.nec.com, riel@redhat.com, rientjes@google.com,
	vbabka@suse.cz, mm-commits@vger.kernel.org, linux-mm@kvack.org
Subject: Re: + mm-thp-avoid-unnecessary-swapin-in-khugepaged.patch added to -mm tree
Date: Tue, 17 May 2016 14:31:14 +0300	[thread overview]
Message-ID: <20160517113114.GC9540@node.shutemov.name> (raw)
In-Reply-To: <20160517090254.GE14453@dhcp22.suse.cz>

On Tue, May 17, 2016 at 11:02:54AM +0200, Michal Hocko wrote:
> On Tue 17-05-16 09:58:15, Michal Hocko wrote:
> > On Thu 28-04-16 17:19:21, Michal Hocko wrote:
> > > On Wed 27-04-16 14:17:20, Andrew Morton wrote:
> > > [...]
> > > > @@ -2484,7 +2485,14 @@ static void collapse_huge_page(struct mm
> > > >  		goto out;
> > > >  	}
> > > >  
> > > > -	__collapse_huge_page_swapin(mm, vma, address, pmd);
> > > > +	swap = get_mm_counter(mm, MM_SWAPENTS);
> > > > +	curr_allocstall = sum_vm_event(ALLOCSTALL);
> > > > +	/*
> > > > +	 * When system under pressure, don't swapin readahead.
> > > > +	 * So that avoid unnecessary resource consuming.
> > > > +	 */
> > > > +	if (allocstall == curr_allocstall && swap != 0)
> > > > +		__collapse_huge_page_swapin(mm, vma, address, pmd);
> > > >  
> > > >  	anon_vma_lock_write(vma->anon_vma);
> > > >  
> > > 
> > > I have mentioned that before already but this seems like a rather weak
> > > heuristic. Don't we really rather teach __collapse_huge_page_swapin
> > > (resp. do_swap_page) do to an optimistic GFP_NOWAIT allocations and
> > > back off under the memory pressure?
> > 
> > I gave it a try and it doesn't seem really bad. Untested and I might
> > have missed something really obvious but what do you think about this
> > approach rather than relying on ALLOCSTALL which is really weak
> > heuristic:
> 
> Ups forgot to add mm/internal.h to the git index
> ---
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 87f09dc986ab..1a4d4c807d92 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -2389,7 +2389,8 @@ static void __collapse_huge_page_swapin(struct mm_struct *mm,
>  		swapped_in++;
>  		ret = do_swap_page(mm, vma, _address, pte, pmd,
>  				   FAULT_FLAG_ALLOW_RETRY|FAULT_FLAG_RETRY_NOWAIT,
> -				   pteval);
> +				   pteval,
> +				   GFP_HIGHUSER_MOVABLE | ~__GFP_DIRECT_RECLAIM);

Why only direct recliam? I'm not sure if triggering kswapd is justified
for swapin. Maybe ~__GFP_RECLAIM?

That said, I like the approach. ALLOCSTALL approach has locking issue[1].

[1] http://lkml.kernel.org/r/20160505013245.GB10429@yexl-desktop

-- 
 Kirill A. Shutemov

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>