Re: [PATCH 28 of 28] memcg huge memory

[Balbir Singh <balbir@linux.vnet.ibm.com>]

* Andrea Arcangeli <aarcange@redhat.com> [2009-12-17 19:00:31]:

> From: Andrea Arcangeli <aarcange@redhat.com>
> 
> Add memcg charge/uncharge to hugepage faults in huge_memory.c.
> 
> Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
> ---
> 
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -207,6 +207,7 @@ static int __do_huge_anonymous_page(stru
>  	VM_BUG_ON(!PageCompound(page));
>  	pgtable = pte_alloc_one(mm, address);
>  	if (unlikely(!pgtable)) {
> +		mem_cgroup_uncharge_page(page);
>  		put_page(page);
>  		return VM_FAULT_OOM;
>  	}
> @@ -218,6 +219,7 @@ static int __do_huge_anonymous_page(stru
> 
>  	spin_lock(&mm->page_table_lock);
>  	if (unlikely(!pmd_none(*pmd))) {
> +		mem_cgroup_uncharge_page(page);
>  		put_page(page);
>  		pte_free(mm, pgtable);
>  	} else {
> @@ -251,6 +253,10 @@ int do_huge_anonymous_page(struct mm_str
>  				   HPAGE_ORDER);
>  		if (unlikely(!page))
>  			goto out;
> +		if (unlikely(mem_cgroup_newpage_charge(page, mm, GFP_KERNEL))) {
> +			put_page(page);
> +			goto out;
> +		}
> 
>  		return __do_huge_anonymous_page(mm, vma,
>  						address, pmd,
> @@ -379,9 +385,16 @@ int do_huge_wp_page(struct mm_struct *mm
>  		for (i = 0; i < HPAGE_NR; i++) {
>  			pages[i] = alloc_page_vma(GFP_HIGHUSER_MOVABLE,
>  						  vma, address);
> -			if (unlikely(!pages[i])) {
> -				while (--i >= 0)
> +			if (unlikely(!pages[i] ||
> +				     mem_cgroup_newpage_charge(pages[i],
> +							       mm,
> +							       GFP_KERNEL))) {
> +				if (pages[i])
>  					put_page(pages[i]);
> +				while (--i >= 0) {
> +					mem_cgroup_uncharge_page(pages[i]);
> +					put_page(pages[i]);
> +				}
>  				kfree(pages);
>  				ret |= VM_FAULT_OOM;
>  				goto out;
> @@ -439,15 +452,21 @@ int do_huge_wp_page(struct mm_struct *mm
>  		goto out;
>  	}
> 
> +	if (unlikely(mem_cgroup_newpage_charge(new_page, mm, GFP_KERNEL))) {
> +		put_page(new_page);
> +		ret |= VM_FAULT_OOM;
> +		goto out;
> +	}
>  	copy_huge_page(new_page, page, haddr, vma, HPAGE_NR);
>  	__SetPageUptodate(new_page);
> 
>  	smp_wmb();
> 
>  	spin_lock(&mm->page_table_lock);
> -	if (unlikely(!pmd_same(*pmd, orig_pmd)))
> +	if (unlikely(!pmd_same(*pmd, orig_pmd))) {
> +		mem_cgroup_uncharge_page(new_page);
>  		put_page(new_page);
> -	else {
> +	} else {
>  		pmd_t entry;
>  		entry = mk_pmd(new_page, vma->vm_page_prot);
>  		entry = maybe_pmd_mkwrite(pmd_mkdirty(entry), vma);
> @@ -466,8 +485,10 @@ out:
>  	return ret;
> 
>  out_free_pages:
> -	for (i = 0; i < HPAGE_NR; i++)
> +	for (i = 0; i < HPAGE_NR; i++) {
> +		mem_cgroup_uncharge_page(pages[i]);
>  		put_page(pages[i]);
> +	}
>  	kfree(pages);
>  	goto out_unlock;
>  }
>

Charging huge pages might be OK, but I wonder if we should create a
separate counter since hugepages are not reclaimable.  I am yet to
look at the complete series, does this series make hugepages
reclaimable? Could you please update Documentation/cgroups/memcg* as
well.

-- 
	Balbir

--
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>