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, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxxx For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>