lkml.org 
[lkml]   [2008]   [Jun]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC 5/5] Memory controller soft limit reclaim on contention
    On Fri, Jun 27, 2008 at 8:19 AM, Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
    > +/*
    > + * Create a heap of memory controller structures. The heap is reverse
    > + * sorted by size. This heap is used for implementing soft limits. Our
    > + * current heap implementation does not allow dynamic heap updates, but
    > + * eventually, the costliest controller (over it's soft limit should

    it's -> its

    > + old_mem = heap_insert(&mem_cgroup_heap, mem,
    > + HEAP_REP_LEAF);
    > + mem->on_heap = 1;
    > + if (old_mem)
    > + old_mem->on_heap = 0;

    Maybe a comment here that mem might == old_mem?

    > + * When the soft limit is exceeded, look through the heap and start
    > + * reclaiming from all groups over thier soft limit

    thier -> their

    > + if (!res_counter_check_under_soft_limit(&mem->res)) {
    > + /*
    > + * The current task might already be over it's soft
    > + * limit and trying to aggressively grow. We check to
    > + * see if it the memory group associated with the
    > + * current task is on the heap when the current group
    > + * is over it's soft limit. If not, we add it
    > + */
    > + if (!mem->on_heap) {
    > + struct mem_cgroup *old_mem;
    > +
    > + old_mem = heap_insert(&mem_cgroup_heap, mem,
    > + HEAP_REP_LEAF);
    > + mem->on_heap = 1;
    > + if (old_mem)
    > + old_mem->on_heap = 0;
    > + }
    > + }

    This and the other similar code for adding to the heap should be
    refactored into a separate function.

    >
    > +static int mem_cgroup_compare_soft_limits(void *p1, void *p2)
    > +{
    > + struct mem_cgroup *mem1 = (struct mem_cgroup *)p1;
    > + struct mem_cgroup *mem2 = (struct mem_cgroup *)p2;
    > + unsigned long long delta1, delta2;
    > +
    > + delta1 = res_counter_soft_limit_delta(&mem1->res);
    > + delta2 = res_counter_soft_limit_delta(&mem2->res);
    > +
    > + return delta1 > delta2;
    > +}

    This isn't a valid comparator, since it isn't a constant function of
    its two input pointers - calling mem_cgroup_compare_soft_limits(m1,
    m2) can give different results at different times. So your heap
    invariant will become invalid over time.

    I think if you want to do this, you're going to need to periodically
    take a snapshot of each cgroup's excess and use that snapshot in the
    comparator; whenever you update the snapshots, you'll need to restore
    the heap invariant.

    Paul


    \
     
     \ /
      Last update: 2008-06-27 18:11    [W:0.119 / U:58.768 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site