linux-kernel - Re: [patch 01/11] mm: memcg: fix compaction/migration failing due to memcg limits

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 9 Jul 2012 16:15:54 +0200
From:	Michal Hocko <mhocko@...e.cz>
To:	Johannes Weiner <hannes@...xchg.org>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	Hugh Dickins <hughd@...gle.com>,
	David Rientjes <rientjes@...gle.com>, linux-mm@...ck.org,
	cgroups@...r.kernel.org, linux-kernel@...r.kernel.org,
	Ingo Molnar <mingo@...nel.org>
Subject: Re: [patch 01/11] mm: memcg: fix compaction/migration failing due to
 memcg limits

[CCing Ingo for the memcg-devel vs tip/sched/numa inter tree dependency
 - see bellow]

On Thu 05-07-12 02:44:53, Johannes Weiner wrote:
> Compaction (and page migration in general) can currently be hindered
> through pages being owned by memory cgroups that are at their limits
> and unreclaimable.
> 
> The reason is that the replacement page is being charged against the
> limit while the page being replaced is also still charged.  But this
> seems unnecessary, given that only one of the two pages will still be
> in use after migration finishes.
> 
> This patch changes the memcg migration sequence so that the
> replacement page is not charged.  Whatever page is still in use after
> successful or failed migration gets to keep the charge of the page
> that was going to be replaced.

Could you mention the side effect on the stat vs charges discrepancy,
please?

> Reported-by: David Rientjes <rientjes@...gle.com>
> Signed-off-by: Johannes Weiner <hannes@...xchg.org>

Acked-by: Michal Hocko <mhocko@...e.cz>

[...]
> diff --git a/mm/migrate.c b/mm/migrate.c
> index 8137aea..aa06bf4 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
[...]
> @@ -1519,10 +1512,9 @@ migrate_misplaced_page(struct page *page, struct mm_struct *mm, int node)
>  {
>  	struct page *oldpage = page, *newpage;
>  	struct address_space *mapping = page_mapping(page);
> -	struct mem_cgroup *mcg;
> +	struct mem_cgroup *memcg;
>  	unsigned int gfp;
>  	int rc = 0;
> -	int charge = -ENOMEM;
>  
>  	VM_BUG_ON(!PageLocked(page));
>  	VM_BUG_ON(page_mapcount(page));
> @@ -1556,12 +1548,7 @@ migrate_misplaced_page(struct page *page, struct mm_struct *mm, int node)
>  	if (!trylock_page(newpage))
>  		BUG();		/* new page should be unlocked!!! */
>  
> -	// XXX hnaz, is this right?
> -	charge = mem_cgroup_prepare_migration(page, newpage, &mcg, gfp);
> -	if (charge == -ENOMEM) {
> -		rc = charge;
> -		goto out;
> -	}
> +	mem_cgroup_prepare_migration(page, newpage, &memcg);
>  
>  	newpage->index = page->index;
>  	newpage->mapping = page->mapping;
> @@ -1581,11 +1568,9 @@ migrate_misplaced_page(struct page *page, struct mm_struct *mm, int node)
>  		page = newpage;
>  	}
>  
> +	mem_cgroup_end_migration(memcg, oldpage, newpage, !rc);
>  out:
> -	if (!charge)
> -		mem_cgroup_end_migration(mcg, oldpage, newpage, !rc);
> -
> -       if (oldpage != page)
> +	if (oldpage != page)
>                 put_page(oldpage);
>  
>  	if (rc) {

Hmm, this depends on 4783af47 (mm: Migrate misplaced page) from
tip/sched/numa which adds an inter tree dependency which is quite
unfortunate from memcg-devel (aka mmotm git tree) tree POV. 
I can cherry-pick this patch into memcg-devel but I am not sure what
is the merging status of the patch (XXX sounds like it is going to be
updated later). Ingo?

-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/