lkml.org 
[lkml]   [2008]   [Mar]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC][2/3] Account and control virtual address space allocations
    Balbir Singh wrote:
    > Pavel Emelyanov wrote:
    >> Balbir Singh wrote:
    >>> Pavel Emelyanov wrote:
    >>>> [snip]
    >>>>
    >>>>> +int mem_cgroup_update_as(struct mm_struct *mm, long nr_pages)
    >>>>> +{
    >>>>> + int ret = 0;
    >>>>> + struct mem_cgroup *mem;
    >>>>> + if (mem_cgroup_subsys.disabled)
    >>>>> + return ret;
    >>>>> +
    >>>>> + rcu_read_lock();
    >>>>> + mem = rcu_dereference(mm->mem_cgroup);
    >>>>> + css_get(&mem->css);
    >>>>> + rcu_read_unlock();
    >>>>> +
    >>>>> + if (nr_pages > 0) {
    >>>>> + if (res_counter_charge(&mem->as_res, (nr_pages * PAGE_SIZE)))
    >>>>> + ret = 1;
    >>>>> + } else
    >>>>> + res_counter_uncharge(&mem->as_res, (-nr_pages * PAGE_SIZE));
    >>>> No, please, no. Let's make two calls - mem_cgroup_charge_as and mem_cgroup_uncharge_as.
    >>>>
    >>>> [snip]
    >>>>
    >>> Yes, sure :)
    >> Thanks :)
    >>
    >>>>> @@ -1117,6 +1117,9 @@ munmap_back:
    >>>>> }
    >>>>> }
    >>>>>
    >>>>> + if (mem_cgroup_update_as(mm, len >> PAGE_SHIFT))
    >>>>> + return -ENOMEM;
    >>>>> +
    >>>> Why not use existintg cap_vm_enough_memory and co?
    >>>>
    >>> I thought about it and almost used may_expand_vm(), but there is a slight catch
    >>> there. With cap_vm_enough_memory() or security_vm_enough_memory(), they are
    >>> called after total_vm has been calculated. In our case we need to keep the
    >>> cgroups equivalent of total_vm up to date, and we do this in mem_cgorup_update_as.
    >> So? What prevents us from using these hooks? :)
    >
    > 1. We need to account total_vm usage of the task anyway. So why have two places,
    > one for accounting and second for control?

    We still have two of them even placing hooks in each place manually.

    Besides, putting the mem_cgroup_(un)charge_as() in these vm hooks will
    1. save the number of places to patch
    2. help keeping memcgroup consistent in case someone adds more places
    that expand tasks vm (arches, drivers) - in case we have our hooks
    celled from inside vm ones, we won't have to patch more.

    > 2. These hooks are activated for conditionally invoked for vma's with VM_ACCOUNT
    > set.

    This is a good point against. But, wrt my previous comment, can we handle
    this somehow?


    \
     
     \ /
      Last update: 2008-03-17 14:05    [W:0.026 / U:0.580 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site