Johannes Weiner [off-list ref] writes:

On Sat, Jun 09, 2012 at 02:29:59PM +0530, Aneesh Kumar K.V wrote:

quoted

From: "Aneesh Kumar K.V" <redacted>

This adds necessary charge/uncharge calls in the HugeTLB code.  We do
hugetlb cgroup charge in page alloc and uncharge in compound page destructor.

Signed-off-by: Aneesh Kumar K.V <redacted>
---
 mm/hugetlb.c        |   16 +++++++++++++++-
 mm/hugetlb_cgroup.c |    7 +------
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index bf79131..4ca92a9 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c

@@ -628,6 +628,8 @@ static void free_huge_page(struct page *page)
 	BUG_ON(page_mapcount(page));
 
 	spin_lock(&hugetlb_lock);
+	hugetlb_cgroup_uncharge_page(hstate_index(h),
+				     pages_per_huge_page(h), page);

hugetlb_cgroup_uncharge_page() takes the hugetlb_lock, no?

Yes, But this patch also modifies it to not take the lock, because we
hold spin_lock just below in the call site. I didn't want to drop the
lock and take it again.

It's quite hard to review code that is split up like this.  Please
always keep the introduction of new functions in the same patch that
adds the callsite(s).

One of the reason I split the charge/uncharge routines and the callers
in separate patches is to make it easier for review. Irrespective of
the call site charge/uncharge routines should be correct with respect
to locking and other details. What I did in this patch is a small
optimization of avoiding dropping and taking the lock again. May be the
right approach would have been to name it __hugetlb_cgroup_uncharge_page
and make sure the hugetlb_cgroup_uncharge_page still takes spin_lock.
But then we don't have any callers for that.

-aneesh