linux-kernel - Re: [patch 13/13] mm: memcontrol: rewrite uncharge API

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 for Android: free password hash cracker in your pocket

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

Date:	Tue, 15 Jul 2014 17:48:43 -0400
From:	Johannes Weiner <hannes@...xchg.org>
To:	Naoya Horiguchi <n-horiguchi@...jp.nec.com>
Cc:	Michal Hocko <mhocko@...e.cz>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Hugh Dickins <hughd@...gle.com>, Tejun Heo <tj@...nel.org>,
	Vladimir Davydov <vdavydov@...allels.com>, linux-mm@...ck.org,
	cgroups@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [patch 13/13] mm: memcontrol: rewrite uncharge API

On Tue, Jul 15, 2014 at 04:49:53PM -0400, Naoya Horiguchi wrote:
> I feel that these 2 messages have the same cause (just appear differently).
> __add_to_page_cache_locked() (and mem_cgroup_try_charge()) can be called
> for hugetlb, while we avoid calling mem_cgroup_migrate()/mem_cgroup_uncharge()
> for hugetlb. This seems to make page_cgroup of the hugepage inconsistent,
> and results in the bad page bug ("page dumped because: cgroup check failed").
> So maybe some more PageHuge check is necessary around the charging code.

This struck me as odd because I don't remember removing a PageHuge()
call in the charge path and wondered how it worked before my changes:
apparently it just checked PageCompound() in mem_cgroup_charge_file().

So it's not fallout of the new uncharge batching code, but was already
broken during the rewrite of the charge API because then hugetlb pages
entered the charging code.

Anyway, we don't have file-specific charging code anymore, and the
PageCompound() check would have required changing anyway for THP
cache.  So I guess the solution is checking PageHuge() in charge,
uncharge, and migrate for now.  Oh well.

How about this?

diff --git a/mm/filemap.c b/mm/filemap.c
index 9c99d6868a5e..b61194273b56 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -564,9 +564,12 @@ static int __add_to_page_cache_locked(struct page *page,
 	VM_BUG_ON_PAGE(!PageLocked(page), page);
 	VM_BUG_ON_PAGE(PageSwapBacked(page), page);
 
-	error = mem_cgroup_try_charge(page, current->mm, gfp_mask, &memcg);
-	if (error)
-		return error;
+	if (!PageHuge(page)) {
+		error = mem_cgroup_try_charge(page, current->mm,
+					      gfp_mask, &memcg);
+		if (error)
+			return error;
+	}
 
 	error = radix_tree_maybe_preload(gfp_mask & ~__GFP_HIGHMEM);
 	if (error) {
diff --git a/mm/migrate.c b/mm/migrate.c
index 7f5a42403fae..dabed2f08609 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -781,7 +781,8 @@ static int move_to_new_page(struct page *newpage, struct page *page,
 		if (!PageAnon(newpage))
 			newpage->mapping = NULL;
 	} else {
-		mem_cgroup_migrate(page, newpage, false);
+		if (!PageHuge(page))
+			mem_cgroup_migrate(page, newpage, false);
 		if (remap_swapcache)
 			remove_migration_ptes(page, newpage);
 		if (!PageAnon(page))
diff --git a/mm/swap.c b/mm/swap.c
index 3461f2f5be20..97b6ec132398 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -62,12 +62,12 @@ static void __page_cache_release(struct page *page)
 		del_page_from_lru_list(page, lruvec, page_off_lru(page));
 		spin_unlock_irqrestore(&zone->lru_lock, flags);
 	}
-	mem_cgroup_uncharge(page);
 }
 
 static void __put_single_page(struct page *page)
 {
 	__page_cache_release(page);
+	mem_cgroup_uncharge_page(page);
 	free_hot_cold_page(page, false);
 }
 
@@ -75,7 +75,10 @@ static void __put_compound_page(struct page *page)
 {
 	compound_page_dtor *dtor;
 
-	__page_cache_release(page);
+	if (!PageHuge(page)) {
+		__page_cache_release(page);
+		mem_cgroup_uncharge_page(page);
+	}
 	dtor = get_compound_page_dtor(page);
 	(*dtor)(page);
 }
--
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/