lkml.org 
[lkml]   [2009]   [Nov]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH 2/2] memcg : rewrite percpu countings with new interfaces
    From
    Christoph Lameter wrote:
    > On Fri, 6 Nov 2009, KAMEZAWA Hiroyuki wrote:
    >> - __mem_cgroup_stat_reset_safe(cpustat, MEMCG_EVENTS);
    >> + __this_cpu_write(mem->cpustat->count[MEMCG_EVENTS], 0);
    >> ret = true;
    >> }
    >> - put_cpu();
    >> return ret;
    >
    > If you want to use the __this_cpu_xx versions then you need to manage
    > preempt on your own.
    >
    Ah, I see. I understand I haven't understood.

    > You need to keep preempt_disable/enable here because otherwise the per
    > cpu variable zeroed may be on a different cpu than the per cpu variable
    > where you got the value from.
    >
    Thank you. I think I can do well in the next version.


    >> +static s64 mem_cgroup_read_stat(struct mem_cgroup *mem,
    >> + enum mem_cgroup_stat_index idx)
    >> +{
    >> + struct mem_cgroup_stat_cpu *cstat;
    >> + int cpu;
    >> + s64 ret = 0;
    >> +
    >> + for_each_possible_cpu(cpu) {
    >> + cstat = per_cpu_ptr(mem->cpustat, cpu);
    >> + ret += cstat->count[idx];
    >> + }
    >
    > == ret += per_cpu(mem->cpustat->cstat->count[idx], cpu)
    >
    Hmm, Hmm. Will use that.

    >> static void mem_cgroup_swap_statistics(struct mem_cgroup *mem,
    >> bool charge)
    >> {
    >> int val = (charge) ? 1 : -1;
    >> - struct mem_cgroup_stat *stat = &mem->stat;
    >> - struct mem_cgroup_stat_cpu *cpustat;
    >> - int cpu = get_cpu();
    >>
    >> - cpustat = &stat->cpustat[cpu];
    >> - __mem_cgroup_stat_add_safe(cpustat, MEMCG_NR_SWAP, val);
    >> - put_cpu();
    >> + __this_cpu_add(mem->cpustat->count[MEMCG_NR_SWAP], val);
    >> }
    >
    > You do not disable preempt on your own so you have to use
    >
    > this_cpu_add()
    >
    > There is no difference between __this_cpu_add and this_cpu_add on x86 but
    > they will differ on platforms that do not have atomic per cpu
    > instructions. The fallback for this_cpu_add is to protect the add with
    > preempt_disable()/enable. The fallback fro __this_cpu_add is just to rely
    > on the caller to ensure that preempt is disabled somehow.
    >
    Ok.


    >> - /*
    >> - * Preemption is already disabled, we don't need get_cpu()
    >> - */
    >> - cpu = smp_processor_id();
    >> - stat = &mem->stat;
    >> - cpustat = &stat->cpustat[cpu];
    >> -
    >> - __mem_cgroup_stat_add_safe(cpustat, MEMCG_NR_FILE_MAPPED, val);
    >> + __this_cpu_add(mem->cpustat->count[MEMCG_NR_FILE_MAPPED], val);
    >
    > Remove __
    >
    >
    >> @@ -1650,16 +1597,11 @@ static int mem_cgroup_move_account(struc
    >>
    >> page = pc->page;
    >> if (page_mapped(page) && !PageAnon(page)) {
    >> - cpu = smp_processor_id();
    >> /* Update mapped_file data for mem_cgroup "from" */
    >> - stat = &from->stat;
    >> - cpustat = &stat->cpustat[cpu];
    >> - __mem_cgroup_stat_add_safe(cpustat, MEMCG_NR_FILE_MAPPED, -1);
    >> + __this_cpu_dec(from->cpustat->count[MEMCG_NR_FILE_MAPPED]);
    >
    > You can keep it here since the context already has preempt disabled it
    > seems.
    >
    Thank you for kindly review.

    Regards,
    -Kame





    \
     
     \ /
      Last update: 2009-11-06 19:47    [W:0.031 / U:1.016 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site