lkml.org 
[lkml]   [2011]   [Nov]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [patch 4/8] mm: memcg: lookup_page_cgroup (almost) never returns NULL
    On Wed 23-11-11 16:42:27, Johannes Weiner wrote:
    > From: Johannes Weiner <jweiner@redhat.com>
    >
    > Pages have their corresponding page_cgroup descriptors set up before
    > they are used in userspace, and thus managed by a memory cgroup.
    >
    > The only time where lookup_page_cgroup() can return NULL is in the
    > page sanity checking code that executes while feeding pages into the
    > page allocator for the first time.
    >
    > Remove the NULL checks against lookup_page_cgroup() results from all
    > callsites where we know that corresponding page_cgroup descriptors
    > must be allocated.

    OK, shouldn't we add

    diff --git a/mm/page_cgroup.c b/mm/page_cgroup.c
    index 2d123f9..cb93f64 100644
    --- a/mm/page_cgroup.c
    +++ b/mm/page_cgroup.c
    @@ -35,8 +35,7 @@ struct page_cgroup *lookup_page_cgroup(struct page *page)
    struct page_cgroup *base;

    base = NODE_DATA(page_to_nid(page))->node_page_cgroup;
    - if (unlikely(!base))
    - return NULL;
    + BUG_ON(!base);

    offset = pfn - NODE_DATA(page_to_nid(page))->node_start_pfn;
    return base + offset;
    @@ -112,8 +111,7 @@ struct page_cgroup *lookup_page_cgroup(struct page *page)
    unsigned long pfn = page_to_pfn(page);
    struct mem_section *section = __pfn_to_section(pfn);

    - if (!section->page_cgroup)
    - return NULL;
    + BUG_ON(!section->page_cgroup);
    return section->page_cgroup + pfn;
    }

    just to make it explicit?
    >
    > Signed-off-by: Johannes Weiner <jweiner@redhat.com>

    Other than that
    Acked-by: Michal Hocko <mhocko@suse.cz>

    > ---
    > mm/memcontrol.c | 8 ++------
    > 1 files changed, 2 insertions(+), 6 deletions(-)
    >
    > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
    > index d825af9..d4d139a 100644
    > --- a/mm/memcontrol.c
    > +++ b/mm/memcontrol.c
    > @@ -1894,9 +1894,6 @@ void mem_cgroup_update_page_stat(struct page *page,
    > bool need_unlock = false;
    > unsigned long uninitialized_var(flags);
    >
    > - if (unlikely(!pc))
    > - return;
    > -
    > rcu_read_lock();
    > memcg = pc->mem_cgroup;
    > if (unlikely(!memcg || !PageCgroupUsed(pc)))
    > @@ -2669,8 +2666,6 @@ static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm,
    > }
    >
    > pc = lookup_page_cgroup(page);
    > - BUG_ON(!pc); /* XXX: remove this and move pc lookup into commit */
    > -
    > ret = __mem_cgroup_try_charge(mm, gfp_mask, nr_pages, &memcg, oom);
    > if (ret || !memcg)
    > return ret;
    > @@ -2942,7 +2937,7 @@ __mem_cgroup_uncharge_common(struct page *page, enum charge_type ctype)
    > * Check if our page_cgroup is valid
    > */
    > pc = lookup_page_cgroup(page);
    > - if (unlikely(!pc || !PageCgroupUsed(pc)))
    > + if (unlikely(!PageCgroupUsed(pc)))
    > return NULL;
    >
    > lock_page_cgroup(pc);
    > @@ -3326,6 +3321,7 @@ static struct page_cgroup *lookup_page_cgroup_used(struct page *page)
    > struct page_cgroup *pc;
    >
    > pc = lookup_page_cgroup(page);
    > + /* Can be NULL while bootstrapping the page allocator */
    > if (likely(pc) && PageCgroupUsed(pc))
    > return pc;
    > return NULL;
    > --
    > 1.7.6.4
    >

    --
    Michal Hocko
    SUSE Labs
    SUSE LINUX s.r.o.
    Lihovarska 1060/12
    190 00 Praha 9
    Czech Republic


    \
     
     \ /
      Last update: 2011-11-24 10:55    [W:0.296 / U:0.680 seconds]
    ©2003-2014 Jasper Spaans. Advertise on this site