lkml.org 
[lkml]   [2010]   [Dec]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 36 of 66] memcg compound
    Hello Kame,

    On Fri, Nov 19, 2010 at 10:10:41AM +0900, KAMEZAWA Hiroyuki wrote:
    > If there are requirements of big page > 4GB, unsigned long should be used.

    There aren't, it's unthinkable at the moment even 1G (besides it seems
    on some implementations the CPU lacks a real 1G tlb and just prefetch
    the same 2M tlb so making the tlb miss read one less cacheline but in
    practice 1G don't seem to provide any measurable runtime compared to
    2M THP). Things will always be exponential, the benefit provided going
    from 4k to 2M will always be an order of magnitude bigger than the
    benefit from 2M to 1G, no matter how much the hardware is handling the
    1G pages natively.

    > > > int nr_oom_retries = MEM_CGROUP_RECLAIM_RETRIES;
    > > > struct mem_cgroup *mem = NULL;
    > > > int ret;
    > > > - int csize = CHARGE_SIZE;
    > > > + int csize = max(CHARGE_SIZE, (unsigned long) page_size);
    > > >
    >
    > unsigned long here.

    This is to shut off a warning because CHARGE_SIZE is calculated as
    multiple of PAGE_SIZE which is unsigned long. csize was already int
    but unsigned long is not required. Like you point out it's not worth
    batching even pages as small as 2G, so no need of unsigned long.

    > > > @@ -2491,14 +2503,14 @@ __do_uncharge(struct mem_cgroup *mem, co
    > > > if (batch->memcg != mem)
    > > > goto direct_uncharge;
    > > > /* remember freed charge and uncharge it later */
    > > > - batch->bytes += PAGE_SIZE;
    > > > + batch->bytes += page_size;
    >
    > Hmm, isn't it simpler to avoid batched-uncharge when page_size > PAGE_SIZE ?

    As you wish, so I'm changing it like this.

    archs where the pmd is implemented purely in software might actually
    be able to use page sizes smaller than 2M that may make sense to
    batch, but for now if you think this is simpler I'll go for it. We
    need simple.

    diff --git a/mm/memcontrol.c b/mm/memcontrol.c
    --- a/mm/memcontrol.c
    +++ b/mm/memcontrol.c
    @@ -2503,6 +2503,9 @@ __do_uncharge(struct mem_cgroup *mem, co
    if (!batch->do_batch || test_thread_flag(TIF_MEMDIE))
    goto direct_uncharge;

    + if (page_size != PAGE_SIZE)
    + goto direct_uncharge;
    +
    /*
    * In typical case, batch->memcg == mem. This means we can
    * merge a series of uncharges to an uncharge of res_counter.
    @@ -2511,9 +2514,9 @@ __do_uncharge(struct mem_cgroup *mem, co
    if (batch->memcg != mem)
    goto direct_uncharge;
    /* remember freed charge and uncharge it later */
    - batch->bytes += page_size;
    + batch->bytes += PAGE_SIZE;
    if (uncharge_memsw)
    - batch->memsw_bytes += page_size;
    + batch->memsw_bytes += PAGE_SIZE;
    return;
    direct_uncharge:
    res_counter_uncharge(&mem->res, page_size);


    \
     
     \ /
      Last update: 2010-12-14 18:43    [W:0.027 / U:31.556 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site