lkml.org 
[lkml]   [2011]   [Jan]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 2/2] memcg: remove charge variable in unmap_and_move
    Date
    memcg charge/uncharge could be handled by mem_cgroup_[prepare/end]
    migration itself so charge local variable in unmap_and_move lost the role
    since we introduced 01b1ae63c2.

    In addition, the variable name is not good like below.

    int unmap_and_move()
    {
    charge = mem_cgroup_prepare_migration(xxx);
    ..
    BUG_ON(charge); <-- BUG if it is charged?
    ..
    uncharge:
    if (!charge) <-- why do we have to uncharge !charge?
    mem_group_end_migration(xxx);
    ..
    }

    So let's remove unnecessary and confusing variable.

    Suggested-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
    Cc: Balbir Singh <balbir@linux.vnet.ibm.com>
    Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
    Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
    ---
    mm/memcontrol.c | 1 +
    mm/migrate.c | 9 +++------
    2 files changed, 4 insertions(+), 6 deletions(-)

    diff --git a/mm/memcontrol.c b/mm/memcontrol.c
    index 8ab8410..b1b572f 100644
    --- a/mm/memcontrol.c
    +++ b/mm/memcontrol.c
    @@ -2804,6 +2804,7 @@ static inline int mem_cgroup_move_swap_account(swp_entry_t entry,
    /*
    * Before starting migration, account PAGE_SIZE to mem_cgroup that the old
    * page belongs to.
    + * Note : Should not return -EAGAIN. unmap_and_move depends on it.
    */
    int mem_cgroup_prepare_migration(struct page *page,
    struct page *newpage, struct mem_cgroup **ptr)
    diff --git a/mm/migrate.c b/mm/migrate.c
    index 8f0f131..5f2169f 100644
    --- a/mm/migrate.c
    +++ b/mm/migrate.c
    @@ -623,7 +623,6 @@ static int unmap_and_move(new_page_t get_new_page, unsigned long private,
    struct page *newpage = get_new_page(page, private, &result);
    int remap_swapcache = 1;
    int rcu_locked = 0;
    - int charge = 0;
    struct mem_cgroup *mem = NULL;
    struct anon_vma *anon_vma = NULL;

    @@ -638,7 +637,7 @@ static int unmap_and_move(new_page_t get_new_page, unsigned long private,
    if (unlikely(split_huge_page(page)))
    goto move_newpage;

    - /* prepare cgroup just returns 0 or -ENOMEM */
    + /* mem_cgroup_prepare_migration never returns -EAGAIN */
    rc = -EAGAIN;

    if (!trylock_page(page)) {
    @@ -662,8 +661,7 @@ static int unmap_and_move(new_page_t get_new_page, unsigned long private,
    }

    /* charge against new page */
    - charge = mem_cgroup_prepare_migration(page, newpage, &mem);
    - if (charge == -ENOMEM) {
    + if (mem_cgroup_prepare_migration(page, newpage, &mem)) {
    rc = -ENOMEM;
    goto unlock;
    }
    @@ -759,8 +757,7 @@ rcu_unlock:
    if (rcu_locked)
    rcu_read_unlock();
    uncharge:
    - if (!charge)
    - mem_cgroup_end_migration(mem, page, newpage, rc == 0);
    + mem_cgroup_end_migration(mem, page, newpage, rc == 0);
    unlock:
    unlock_page(page);

    --
    1.7.0.4


    \
     
     \ /
      Last update: 2011-01-11 09:55    [W:0.050 / U:67.012 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site