Re: [PATCH 39 of 66] memcg huge memory

Andrea Arcangeli
2010-12-14T17:39:46Z

On Fri, Nov 19, 2010 at 10:19:38AM +0900, KAMEZAWA Hiroyuki wrote:
> On Wed, 03 Nov 2010 16:28:14 +0100
> Andrea Arcangeli <aarcange@redhat.com> wrote:
> > @@ -402,9 +408,15 @@ static int do_huge_pmd_wp_page_fallback(
> > for (i = 0; i < HPAGE_PMD_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]);
> > + }
>
> Maybe you can use batched-uncharge here.
> ==
> mem_cgroup_uncharge_start()
> {
> do loop;
> }
> mem_cgroup_uncharge_end();
> ==
> Then, many atomic ops can be reduced.

Cute!

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -413,10 +413,12 @@ static int do_huge_pmd_wp_page_fallback(
GFP_KERNEL))) {
if (pages[i])
put_page(pages[i]);
+ mem_cgroup_uncharge_start();
while (--i >= 0) {
mem_cgroup_uncharge_page(pages[i]);
put_page(pages[i]);
}
+ mem_cgroup_uncharge_end();
kfree(pages);
ret |= VM_FAULT_OOM;
goto out;

>
>
> > kfree(pages);
> > ret |= VM_FAULT_OOM;
> > goto out;
> > @@ -455,8 +467,10 @@ out:
> >
> > out_free_pages:
> > spin_unlock(&mm->page_table_lock);
> > - for (i = 0; i < HPAGE_PMD_NR; i++)
> > + for (i = 0; i < HPAGE_PMD_NR; i++) {
> > + mem_cgroup_uncharge_page(pages[i]);
> > put_page(pages[i]);
> > + }
>
> here, too.

This is actually a very unlikely path handling a thread race
condition, but I'll add it any way.

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -469,10 +469,12 @@ out:

out_free_pages:
spin_unlock(&mm->page_table_lock);
+ mem_cgroup_uncharge_start();
for (i = 0; i < HPAGE_PMD_NR; i++) {
mem_cgroup_uncharge_page(pages[i]);
put_page(pages[i]);
}
+ mem_cgroup_uncharge_end();
kfree(pages);
goto out;
}

> Hmm...it seems there are no codes for move_account() hugepage in series.
> I think it needs some complicated work to walk page table.

Agreed.