lkml.org 
[lkml]   [2011]   [Feb]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [patch 4/4] memcg: unify charge/uncharge quantities to units of pages
    On Wed,  9 Feb 2011 12:01:53 +0100
    Johannes Weiner <hannes@cmpxchg.org> wrote:

    > There is no clear pattern when we pass a page count and when we pass a
    > byte count that is a multiple of PAGE_SIZE.
    >
    > We never charge or uncharge subpage quantities, so convert it all to
    > page counts.
    >
    > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>

    Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>


    > ---
    > mm/memcontrol.c | 117 +++++++++++++++++++++++++-----------------------------
    > 1 files changed, 54 insertions(+), 63 deletions(-)
    >
    > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
    > index ab5cd3b..bc03f56 100644
    > --- a/mm/memcontrol.c
    > +++ b/mm/memcontrol.c
    > @@ -1083,16 +1083,16 @@ unsigned long mem_cgroup_isolate_pages(unsigned long nr_to_scan,
    > * @mem: the memory cgroup
    > *
    > * Returns the maximum amount of memory @mem can be charged with, in
    > - * bytes.
    > + * pages.
    > */
    > -static unsigned long long mem_cgroup_margin(struct mem_cgroup *mem)
    > +static unsigned long mem_cgroup_margin(struct mem_cgroup *mem)
    > {
    > unsigned long long margin;
    >
    > margin = res_counter_margin(&mem->res);
    > if (do_swap_account)
    > margin = min(margin, res_counter_margin(&mem->memsw));
    > - return margin;
    > + return margin >> PAGE_SHIFT;
    > }
    >
    > static unsigned int get_swappiness(struct mem_cgroup *memcg)
    > @@ -1621,7 +1621,7 @@ EXPORT_SYMBOL(mem_cgroup_update_page_stat);
    > * size of first charge trial. "32" comes from vmscan.c's magic value.
    > * TODO: maybe necessary to use big numbers in big irons.
    > */
    > -#define CHARGE_SIZE (32 * PAGE_SIZE)
    > +#define CHARGE_BATCH 32U
    > struct memcg_stock_pcp {
    > struct mem_cgroup *cached; /* this never be root cgroup */
    > unsigned int nr_pages;
    > @@ -1797,8 +1797,9 @@ enum {
    > };
    >
    > static int __mem_cgroup_do_charge(struct mem_cgroup *mem, gfp_t gfp_mask,
    > - int csize, bool oom_check)
    > + unsigned int nr_pages, bool oom_check)
    > {
    > + unsigned long csize = nr_pages * PAGE_SIZE;
    > struct mem_cgroup *mem_over_limit;
    > struct res_counter *fail_res;
    > unsigned long flags = 0;
    > @@ -1819,14 +1820,13 @@ static int __mem_cgroup_do_charge(struct mem_cgroup *mem, gfp_t gfp_mask,
    > } else
    > mem_over_limit = mem_cgroup_from_res_counter(fail_res, res);
    > /*
    > - * csize can be either a huge page (HPAGE_SIZE), a batch of
    > - * regular pages (CHARGE_SIZE), or a single regular page
    > - * (PAGE_SIZE).
    > + * nr_pages can be either a huge page (HPAGE_PMD_NR), a batch
    > + * of regular pages (CHARGE_BATCH), or a single regular page (1).
    > *
    > * Never reclaim on behalf of optional batching, retry with a
    > * single page instead.
    > */
    > - if (csize == CHARGE_SIZE)
    > + if (nr_pages == CHARGE_BATCH)
    > return CHARGE_RETRY;
    >
    > if (!(gfp_mask & __GFP_WAIT))
    > @@ -1834,7 +1834,7 @@ static int __mem_cgroup_do_charge(struct mem_cgroup *mem, gfp_t gfp_mask,
    >
    > ret = mem_cgroup_hierarchical_reclaim(mem_over_limit, NULL,
    > gfp_mask, flags);
    > - if (mem_cgroup_margin(mem_over_limit) >= csize)
    > + if (mem_cgroup_margin(mem_over_limit) >= nr_pages)
    > return CHARGE_RETRY;
    > /*
    > * Even though the limit is exceeded at this point, reclaim
    > @@ -1845,7 +1845,7 @@ static int __mem_cgroup_do_charge(struct mem_cgroup *mem, gfp_t gfp_mask,
    > * unlikely to succeed so close to the limit, and we fall back
    > * to regular pages anyway in case of failure.
    > */
    > - if (csize == PAGE_SIZE && ret)
    > + if (nr_pages == 1 && ret)
    > return CHARGE_RETRY;
    >
    > /*
    > @@ -1872,12 +1872,12 @@ static int __mem_cgroup_do_charge(struct mem_cgroup *mem, gfp_t gfp_mask,
    > static int __mem_cgroup_try_charge(struct mm_struct *mm,
    > gfp_t gfp_mask,
    > struct mem_cgroup **memcg, bool oom,
    > - int page_size)
    > + unsigned int nr_pages)
    > {
    > + unsigned int batch = max(CHARGE_BATCH, nr_pages);
    > int nr_oom_retries = MEM_CGROUP_RECLAIM_RETRIES;
    > struct mem_cgroup *mem = NULL;
    > int ret;
    > - int csize = max(CHARGE_SIZE, (unsigned long) page_size);
    >
    > /*
    > * Unlike gloval-vm's OOM-kill, we're not in memory shortage
    > @@ -1902,7 +1902,7 @@ again:
    > VM_BUG_ON(css_is_removed(&mem->css));
    > if (mem_cgroup_is_root(mem))
    > goto done;
    > - if (page_size == PAGE_SIZE && consume_stock(mem))
    > + if (nr_pages == 1 && consume_stock(mem))
    > goto done;
    > css_get(&mem->css);
    > } else {
    > @@ -1925,7 +1925,7 @@ again:
    > rcu_read_unlock();
    > goto done;
    > }
    > - if (page_size == PAGE_SIZE && consume_stock(mem)) {
    > + if (nr_pages == 1 && consume_stock(mem)) {
    > /*
    > * It seems dagerous to access memcg without css_get().
    > * But considering how consume_stok works, it's not
    > @@ -1960,13 +1960,12 @@ again:
    > nr_oom_retries = MEM_CGROUP_RECLAIM_RETRIES;
    > }
    >
    > - ret = __mem_cgroup_do_charge(mem, gfp_mask, csize, oom_check);
    > -
    > + ret = __mem_cgroup_do_charge(mem, gfp_mask, batch, oom_check);
    > switch (ret) {
    > case CHARGE_OK:
    > break;
    > case CHARGE_RETRY: /* not in OOM situation but retry */
    > - csize = page_size;
    > + batch = nr_pages;
    > css_put(&mem->css);
    > mem = NULL;
    > goto again;
    > @@ -1987,8 +1986,8 @@ again:
    > }
    > } while (ret != CHARGE_OK);
    >
    > - if (csize > page_size)
    > - refill_stock(mem, (csize - page_size) >> PAGE_SHIFT);
    > + if (batch > nr_pages)
    > + refill_stock(mem, batch - nr_pages);
    > css_put(&mem->css);
    > done:
    > *memcg = mem;
    > @@ -2069,10 +2068,8 @@ static void __mem_cgroup_commit_charge(struct mem_cgroup *mem,
    > struct page *page,
    > struct page_cgroup *pc,
    > enum charge_type ctype,
    > - int page_size)
    > + unsigned int nr_pages)
    > {
    > - int nr_pages = page_size >> PAGE_SHIFT;
    > -
    > lock_page_cgroup(pc);
    > if (unlikely(PageCgroupUsed(pc))) {
    > unlock_page_cgroup(pc);
    > @@ -2165,11 +2162,11 @@ void mem_cgroup_split_huge_fixup(struct page *head, struct page *tail)
    > * @from: mem_cgroup which the page is moved from.
    > * @to: mem_cgroup which the page is moved to. @from != @to.
    > * @uncharge: whether we should call uncharge and css_put against @from.
    > - * @charge_size: number of bytes to charge (regular or huge page)
    > + * @nr_pages: number of regular pages (>1 for huge pages)
    > *
    > * The caller must confirm following.
    > * - page is not on LRU (isolate_page() is useful.)
    > - * - compound_lock is held when charge_size > PAGE_SIZE
    > + * - compound_lock is held when nr_pages > 1
    > *
    > * This function doesn't do "charge" nor css_get to new cgroup. It should be
    > * done by a caller(__mem_cgroup_try_charge would be usefull). If @uncharge is
    > @@ -2178,9 +2175,8 @@ void mem_cgroup_split_huge_fixup(struct page *head, struct page *tail)
    > */
    > static int mem_cgroup_move_account(struct page *page, struct page_cgroup *pc,
    > struct mem_cgroup *from, struct mem_cgroup *to,
    > - bool uncharge, int charge_size)
    > + bool uncharge, unsigned int nr_pages)
    > {
    > - int nr_pages = charge_size >> PAGE_SHIFT;
    > unsigned long flags;
    > int ret;
    >
    > @@ -2193,7 +2189,7 @@ static int mem_cgroup_move_account(struct page *page, struct page_cgroup *pc,
    > * hold it.
    > */
    > ret = -EBUSY;
    > - if (charge_size > PAGE_SIZE && !PageTransHuge(page))
    > + if (nr_pages > 1 && !PageTransHuge(page))
    > goto out;
    >
    > lock_page_cgroup(pc);
    > @@ -2251,7 +2247,7 @@ static int mem_cgroup_move_parent(struct page *page,
    > struct cgroup *cg = child->css.cgroup;
    > struct cgroup *pcg = cg->parent;
    > struct mem_cgroup *parent;
    > - int page_size = PAGE_SIZE;
    > + unsigned int nr_pages;
    > unsigned long flags;
    > int ret;
    >
    > @@ -2265,23 +2261,21 @@ static int mem_cgroup_move_parent(struct page *page,
    > if (isolate_lru_page(page))
    > goto put;
    >
    > - if (PageTransHuge(page))
    > - page_size = HPAGE_SIZE;
    > + nr_pages = hpage_nr_pages(page);
    >
    > parent = mem_cgroup_from_cont(pcg);
    > - ret = __mem_cgroup_try_charge(NULL, gfp_mask,
    > - &parent, false, page_size);
    > + ret = __mem_cgroup_try_charge(NULL, gfp_mask, &parent, false, nr_pages);
    > if (ret || !parent)
    > goto put_back;
    >
    > - if (page_size > PAGE_SIZE)
    > + if (nr_pages > 1)
    > flags = compound_lock_irqsave(page);
    >
    > - ret = mem_cgroup_move_account(page, pc, child, parent, true, page_size);
    > + ret = mem_cgroup_move_account(page, pc, child, parent, true, nr_pages);
    > if (ret)
    > - mem_cgroup_cancel_charge(parent, page_size >> PAGE_SHIFT);
    > + mem_cgroup_cancel_charge(parent, nr_pages);
    >
    > - if (page_size > PAGE_SIZE)
    > + if (nr_pages > 1)
    > compound_unlock_irqrestore(page, flags);
    > put_back:
    > putback_lru_page(page);
    > @@ -2301,13 +2295,13 @@ static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm,
    > gfp_t gfp_mask, enum charge_type ctype)
    > {
    > struct mem_cgroup *mem = NULL;
    > - int page_size = PAGE_SIZE;
    > + unsigned int nr_pages = 1;
    > struct page_cgroup *pc;
    > bool oom = true;
    > int ret;
    >
    > if (PageTransHuge(page)) {
    > - page_size <<= compound_order(page);
    > + nr_pages <<= compound_order(page);
    > VM_BUG_ON(!PageTransHuge(page));
    > /*
    > * Never OOM-kill a process for a huge page. The
    > @@ -2319,11 +2313,11 @@ static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm,
    > pc = lookup_page_cgroup(page);
    > BUG_ON(!pc); /* XXX: remove this and move pc lookup into commit */
    >
    > - ret = __mem_cgroup_try_charge(mm, gfp_mask, &mem, oom, page_size);
    > + ret = __mem_cgroup_try_charge(mm, gfp_mask, &mem, oom, nr_pages);
    > if (ret || !mem)
    > return ret;
    >
    > - __mem_cgroup_commit_charge(mem, page, pc, ctype, page_size);
    > + __mem_cgroup_commit_charge(mem, page, pc, ctype, nr_pages);
    > return 0;
    > }
    >
    > @@ -2439,13 +2433,13 @@ int mem_cgroup_try_charge_swapin(struct mm_struct *mm,
    > if (!mem)
    > goto charge_cur_mm;
    > *ptr = mem;
    > - ret = __mem_cgroup_try_charge(NULL, mask, ptr, true, PAGE_SIZE);
    > + ret = __mem_cgroup_try_charge(NULL, mask, ptr, true, 1);
    > css_put(&mem->css);
    > return ret;
    > charge_cur_mm:
    > if (unlikely(!mm))
    > mm = &init_mm;
    > - return __mem_cgroup_try_charge(mm, mask, ptr, true, PAGE_SIZE);
    > + return __mem_cgroup_try_charge(mm, mask, ptr, true, 1);
    > }
    >
    > static void
    > @@ -2461,7 +2455,7 @@ __mem_cgroup_commit_charge_swapin(struct page *page, struct mem_cgroup *ptr,
    > cgroup_exclude_rmdir(&ptr->css);
    > pc = lookup_page_cgroup(page);
    > mem_cgroup_lru_del_before_commit_swapcache(page);
    > - __mem_cgroup_commit_charge(ptr, page, pc, ctype, PAGE_SIZE);
    > + __mem_cgroup_commit_charge(ptr, page, pc, ctype, 1);
    > mem_cgroup_lru_add_after_commit_swapcache(page);
    > /*
    > * Now swap is on-memory. This means this page may be
    > @@ -2515,10 +2509,11 @@ void mem_cgroup_cancel_charge_swapin(struct mem_cgroup *mem)
    >
    > static void
    > __do_uncharge(struct mem_cgroup *mem, const enum charge_type ctype,
    > - int page_size)
    > + unsigned int nr_pages)
    > {
    > struct memcg_batch_info *batch = NULL;
    > bool uncharge_memsw = true;
    > +
    > /* If swapout, usage of swap doesn't decrease */
    > if (!do_swap_account || ctype == MEM_CGROUP_CHARGE_TYPE_SWAPOUT)
    > uncharge_memsw = false;
    > @@ -2542,7 +2537,7 @@ __do_uncharge(struct mem_cgroup *mem, const enum charge_type ctype,
    > if (!batch->do_batch || test_thread_flag(TIF_MEMDIE))
    > goto direct_uncharge;
    >
    > - if (page_size != PAGE_SIZE)
    > + if (nr_pages > 1)
    > goto direct_uncharge;
    >
    > /*
    > @@ -2558,9 +2553,9 @@ __do_uncharge(struct mem_cgroup *mem, const enum charge_type ctype,
    > batch->memsw_nr_pages++;
    > return;
    > direct_uncharge:
    > - res_counter_uncharge(&mem->res, page_size);
    > + res_counter_uncharge(&mem->res, nr_pages * PAGE_SIZE);
    > if (uncharge_memsw)
    > - res_counter_uncharge(&mem->memsw, page_size);
    > + res_counter_uncharge(&mem->memsw, nr_pages * PAGE_SIZE);
    > if (unlikely(batch->memcg != mem))
    > memcg_oom_recover(mem);
    > return;
    > @@ -2572,10 +2567,9 @@ direct_uncharge:
    > static struct mem_cgroup *
    > __mem_cgroup_uncharge_common(struct page *page, enum charge_type ctype)
    > {
    > - int count;
    > - struct page_cgroup *pc;
    > struct mem_cgroup *mem = NULL;
    > - int page_size = PAGE_SIZE;
    > + unsigned int nr_pages = 1;
    > + struct page_cgroup *pc;
    >
    > if (mem_cgroup_disabled())
    > return NULL;
    > @@ -2584,11 +2578,9 @@ __mem_cgroup_uncharge_common(struct page *page, enum charge_type ctype)
    > return NULL;
    >
    > if (PageTransHuge(page)) {
    > - page_size <<= compound_order(page);
    > + nr_pages <<= compound_order(page);
    > VM_BUG_ON(!PageTransHuge(page));
    > }
    > -
    > - count = page_size >> PAGE_SHIFT;
    > /*
    > * Check if our page_cgroup is valid
    > */
    > @@ -2621,7 +2613,7 @@ __mem_cgroup_uncharge_common(struct page *page, enum charge_type ctype)
    > break;
    > }
    >
    > - mem_cgroup_charge_statistics(mem, PageCgroupCache(pc), -count);
    > + mem_cgroup_charge_statistics(mem, PageCgroupCache(pc), -nr_pages);
    >
    > ClearPageCgroupUsed(pc);
    > /*
    > @@ -2642,7 +2634,7 @@ __mem_cgroup_uncharge_common(struct page *page, enum charge_type ctype)
    > mem_cgroup_get(mem);
    > }
    > if (!mem_cgroup_is_root(mem))
    > - __do_uncharge(mem, ctype, page_size);
    > + __do_uncharge(mem, ctype, nr_pages);
    >
    > return mem;
    >
    > @@ -2834,8 +2826,8 @@ static inline int mem_cgroup_move_swap_account(swp_entry_t entry,
    > int mem_cgroup_prepare_migration(struct page *page,
    > struct page *newpage, struct mem_cgroup **ptr, gfp_t gfp_mask)
    > {
    > - struct page_cgroup *pc;
    > struct mem_cgroup *mem = NULL;
    > + struct page_cgroup *pc;
    > enum charge_type ctype;
    > int ret = 0;
    >
    > @@ -2891,7 +2883,7 @@ int mem_cgroup_prepare_migration(struct page *page,
    > return 0;
    >
    > *ptr = mem;
    > - ret = __mem_cgroup_try_charge(NULL, gfp_mask, ptr, false, PAGE_SIZE);
    > + ret = __mem_cgroup_try_charge(NULL, gfp_mask, ptr, false, 1);
    > css_put(&mem->css);/* drop extra refcnt */
    > if (ret || *ptr == NULL) {
    > if (PageAnon(page)) {
    > @@ -2918,7 +2910,7 @@ int mem_cgroup_prepare_migration(struct page *page,
    > ctype = MEM_CGROUP_CHARGE_TYPE_CACHE;
    > else
    > ctype = MEM_CGROUP_CHARGE_TYPE_SHMEM;
    > - __mem_cgroup_commit_charge(mem, page, pc, ctype, PAGE_SIZE);
    > + __mem_cgroup_commit_charge(mem, page, pc, ctype, 1);
    > return ret;
    > }
    >
    > @@ -4572,8 +4564,7 @@ one_by_one:
    > batch_count = PRECHARGE_COUNT_AT_ONCE;
    > cond_resched();
    > }
    > - ret = __mem_cgroup_try_charge(NULL, GFP_KERNEL, &mem, false,
    > - PAGE_SIZE);
    > + ret = __mem_cgroup_try_charge(NULL, GFP_KERNEL, &mem, false, 1);
    > if (ret || !mem)
    > /* mem_cgroup_clear_mc() will do uncharge later */
    > return -ENOMEM;
    > @@ -4918,8 +4909,8 @@ retry:
    > if (isolate_lru_page(page))
    > goto put;
    > pc = lookup_page_cgroup(page);
    > - if (!mem_cgroup_move_account(page, pc,
    > - mc.from, mc.to, false, PAGE_SIZE)) {
    > + if (!mem_cgroup_move_account(page, pc, mc.from,
    > + mc.to, false, 1)) {
    > mc.precharge--;
    > /* we uncharge from mc.from later. */
    > mc.moved_charge++;
    > --
    > 1.7.4
    >
    >



    \
     
     \ /
      Last update: 2011-02-10 01:03    [W:3.623 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site