lkml.org 
[lkml]   [2012]   [Jun]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH -V8 12/16] hugetlb/cgroup: Add support for cgroup removal
    On Mon 11-06-12 15:10:20, Aneesh Kumar K.V wrote:
    > Michal Hocko <mhocko@suse.cz> writes:
    [...]
    > >> +static int hugetlb_cgroup_move_parent(int idx, struct cgroup *cgroup,
    > >> + struct page *page)
    > >
    > > deserves a comment about the locking (needs to be called with
    > > hugetlb_lock).
    >
    > will do
    >
    > >
    > >> +{
    > >> + int csize;
    > >> + struct res_counter *counter;
    > >> + struct res_counter *fail_res;
    > >> + struct hugetlb_cgroup *page_hcg;
    > >> + struct hugetlb_cgroup *h_cg = hugetlb_cgroup_from_cgroup(cgroup);
    > >> + struct hugetlb_cgroup *parent = parent_hugetlb_cgroup(cgroup);
    > >> +
    > >> + if (!get_page_unless_zero(page))
    > >> + goto out;
    > >> +
    > >> + page_hcg = hugetlb_cgroup_from_page(page);
    > >> + /*
    > >> + * We can have pages in active list without any cgroup
    > >> + * ie, hugepage with less than 3 pages. We can safely
    > >> + * ignore those pages.
    > >> + */
    > >> + if (!page_hcg || page_hcg != h_cg)
    > >> + goto err_out;
    > >
    > > How can we have page_hcg != NULL && page_hcg != h_cg?
    >
    > pages belonging to other cgroup ?

    OK, I've forgot that you are iterating over all active huge pages in
    hugetlb_cgroup_pre_destroy. What prevents you from doing the filtering
    in the caller?
    I am also wondering why you need to play with the page reference
    counting here. You are under hugetlb_lock so the page cannot disappear
    in the meantime or am I missing something?
    --
    Michal Hocko
    SUSE Labs
    SUSE LINUX s.r.o.
    Lihovarska 1060/12
    190 00 Praha 9
    Czech Republic


    \
     
     \ /
      Last update: 2012-06-11 16:03    [W:0.027 / U:0.244 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site