On Tue, 02 Oct 2007 21:04:40 +0530
Balbir Singh <[EMAIL PROTECTED]> wrote:
> KAMEZAWA Hiroyuki wrote:
> > While using memory control cgroup, page-migration under it works as
> > following.
> > ==
> > 1. uncharge all refs at try to unmap.
> > 2. charge regs again remove_migration_ptes()
> > ==
> > This is simple but has following problems.
> > ==
> > The page is uncharged and chaged back again if *mapped*.
> > - This means that cgroup before migraion can be different from one after
> > migraion
>
> >From the test case mentioned earlier, this happens because the task has
> moved from one cgroup to another, right?
Ah, yes.
> > And migration can migrate *not mapped* pages in future by
> > migration-by-kernel
> > driven by memory-unplug and defragment-by-migration at el.
> >
> > This patch tries to keep memory cgroup at page migration by increasing
> > one refcnt during it. 3 functions are added.
> > mem_cgroup_prepare_migration() --- increase refcnt of page->page_cgroup
> > mem_cgroup_end_migration() --- decrease refcnt of page->page_cgroup
> > mem_cgroup_page_migration() --- copy page->page_cgroup from old page to
> > new page.
> >
> > Obviously, mem_cgroup_isolate_pages() and this page migration, which
> > copies page_cgroup from old page to new page, has race.
> >
> > There seem to be 3 ways for avoiding this race.
> > A. take mem_group->lock while mem_cgroup_page_migration().
> > B. isolate pc from mem_cgroup's LRU when we isolate page from zone's LRU.
> > C. ignore non-LRU page at mem_cgroup_isolate_pages().
> >
> > This patch uses method (C.) and modifes mem_cgroup_isolate_pages() igonres
> > !PageLRU pages.
> >
>
> The page(s) is(are) !PageLRU only during page migration right?
>
Hmm...!PageLRU() means that page is not on LRU.
Then, kswapd can remove a page from LRU.
> > - if (page_zone(page) != z)
> > + if (page_zone(page) != z || !PageLRU(page)) {
>
> I would prefer to do unlikely(!PageLRU(page)), since most of the
> times the page is not under migration
>
I see.
> > + /* Skip this */
> > + /* Don't decrease scan here for avoiding dead lock */
>
> Could we merge the two comments to one block comment?
>
will do
> > continue;
> > + }
> >
> > /*
> > * Check if the meta page went away from under us
> > @@ -417,8 +424,14 @@ void mem_cgroup_uncharge(struct page_cgr
> > return;
> >
> > if (atomic_dec_and_test(&pc->ref_cnt)) {
> > +retry:
> > page = pc->page;
> > lock_page_cgroup(page);
> > + /* migration occur ? */
> > + if (page_get_page_cgroup(page) != pc) {
> > + unlock_page_cgroup(page);
> > + goto retry;
>
> Shouldn't we check if page_get_page_cgroup(page) returns
> NULL, if so, unlock and return?
Hmm, I think page_get_page_cgroup(page) != pc covers it. pc is not NULL.
Thanks,
-Kame
_______________________________________________
Containers mailing list
[EMAIL PROTECTED]
https://lists.linux-foundation.org/mailman/listinfo/containers
_______________________________________________
Devel mailing list
Devel@openvz.org
https://openvz.org/mailman/listinfo/devel
