lkml.org 
[lkml]   [2012]   [Mar]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v3 2/2] memcg: avoid THP split in task migration
    On Fri, 9 Mar 2012, KAMEZAWA Hiroyuki wrote:
    > > +
    > > + page = pmd_page(pmd);
    > > + VM_BUG_ON(!page || !PageHead(page));
    > > + if (!move_anon() || page_mapcount(page) != 1)
    > > + return 0;
    >
    > Could you add this ?
    > ==
    > static bool move_check_shared_map(struct page *page)
    > {
    > /*
    > * Handling of shared pages between processes is a big trouble in memcg.
    > * Now, we never move shared-mapped pages between memcg at 'task' moving because
    > * we have no hint which task the page is really belongs to. For example,
    > * When a task does "fork()-> move to the child other group -> exec()", the charges
    > * should be stay in the original cgroup.
    > * So, check mapcount to determine we can move or not.
    > */
    > return page_mapcount(page) != 1;
    > }

    That's a helpful elucidation, thank you. However...

    That is not how it has actually been behaving for the last 18 months
    (because of the "> 2" bug), so in practice you are asking for a change
    in behaviour there.

    And it's not how it has been and continues to behave with file pages.

    Isn't getting that behaviour in fork-move-exec just a good reason not
    to set move_charge_at_immigrate?

    I think there are other scenarios where you do want all the pages to
    move if move_charge_at_immigrate: and that's certainly easier to
    describe and to understand and to code.

    But if you do insist on not moving the shared, then it needs to involve
    something like mem_cgroup_count_swap_user() on PageSwapCache pages,
    rather than just the bare page_mapcount().

    I'd rather delete than add code here!

    Hugh


    \
     
     \ /
      Last update: 2012-03-09 03:37    [W:0.024 / U:36.000 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site