> > > @@ -2517,65 +2519,70 @@ int mem_cgroup_prepare_migration(struct
> > > css_get(&mem->css);
> > > }
> > > unlock_page_cgroup(pc);
> > > -
> > > - if (mem) {
> > > - ret = __mem_cgroup_try_charge(NULL, GFP_KERNEL, &mem, false);
> > > - css_put(&mem->css);
> > > - }
> > > - *ptr = mem;
> > > + /*
> > > + * If the page is uncharged before migration (removed from radix-tree)
> > > + * we return here.
> > > + */
> > > + if (!mem)
> > > + return 0;
> > > + ret = __mem_cgroup_try_charge(NULL, GFP_KERNEL, &mem, false);
> > > + css_put(&mem->css); /* drop extra refcnt */
> > it should be:
> >
> > *ptr = mem;
> > ret = __mem_cgroup_try_charge(NULL, GFP_KERNEL, ptr, false);
> > css_put(&mem->css);
> >
> > as Andrea has fixed already.
> >
> Ah, yes. I'll rebase this onto Andrea's fix.
>
>
>
> > > + if (ret)
We should check "if (ret || !*ptr)" not to do commit in !*ptr case.
> > > + * Considering ANON pages, we can't depend on lock_page.
> > > + * If a page may be unmapped before it's remapped, new page's
> > > + * mapcount will not increase. (case that mapcount 0->1 never occur.)
> > > + * PageCgroupUsed() and SwapCache checks will be done.
> > > + *
> > > + * Once mapcount goes to 1, our hook to page_remove_rmap will do
> > > + * enough jobs.
> > > + */
> > > + if (PageAnon(used) && !page_mapped(used))
> > > + mem_cgroup_uncharge_page(used);
> > mem_cgroup_uncharge_page() does the same check :)
> >
> Ok. I'll fix.
>
Considering more, we'd better to check PageAnon() at least not to call
mem_cgroup_uncharge_page() for cache page.
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/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>