Re: [PATCH 4.2 25/54] memcg: convert threshold to bytes

From: Michal Hocko
Date: Sun Oct 25 2015 - 12:32:19 EST


On Sat 24-10-15 14:46:58, Ben Hutchings wrote:
[...]
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -3687,6 +3687,7 @@ static int __mem_cgroup_usage_register_e
> >  > > ret = page_counter_memparse(args, "-1", &threshold);
> >  > > if (ret)
> >  > > > return ret;
> > +> > threshold <<= PAGE_SHIFT;
> >  
> >  > > mutex_lock(&memcg->thresholds_lock);
> >  
>
> mem_cgroup_usage() returns a u64 and I think that the types of
> threshold and mem_cgroup_threshold::threshold also need be changed to
> u64 to avoid overflow on large 32-bit systems.

You are absolutely right! I have compltely missed that 3e32cb2e0a12 has
changed the type as well. Should have noticed that during the review.

The whole thing is just way too confusing. All the tracking is done in
page units yet tresholds are in bytes. This only calls for troubles. The
patch below simply turns thresholds to page units as well. I hope I
haven't screwed anything, I didn't get to more than compile test it.
---