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

[Daisuke Nishimura]

On Mon, 29 Nov 2010 17:11:03 +0100
Andrea Arcangeli <aarcange@xxxxxxxxxx> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/