Re: [PATCH 53 of 66] add numa awareness to hugepage allocations

Daisuke Nishimura
2010-11-30T00:47:18Z

On Mon, 29 Nov 2010 17:11:03 +0100
Andrea Arcangeli <aarcange@redhat.com> wrote:

> On Mon, Nov 29, 2010 at 02:38:01PM +0900, Daisuke Nishimura wrote:
> > I think this should be:
> >
> > if (unlikely(mem_cgroup_newpage_charge(new_page, mm, GFP_KERNEL))) {
> > #ifdef CONFIG_NUMA
> > put_page(new_page);
> > #endif
> > goto out;
> > }
>
> Hmm no, the change you suggest would generate memory corruption with
> use after free.

I'm sorry if I miss something, "new_page" will be reused in !CONFIG_NUMA case
as you say, but, in CONFIG_NUMA case, it is allocated in this function
(collapse_huge_page()) by alloc_hugepage_vma(), and is not freed when memcg's
charge failed.
Actually, we do in collapse_huge_page():
if (unlikely(!isolated)) {
...
#ifdef CONFIG_NUMA
put_page(new_page);
#endif
goto out;
}
later. I think we need a similar logic in memcg's failure path too.

Thanks,
Daisuke Nishimura.