Messages in this thread | | | Subject | Re: [RFC][PATCH 3/5] memcg: Disable preemption in memcg_check_events() | From | Steven Rostedt <> | Date | Tue, 20 Sep 2011 10:33:17 -0400 |
| |
On Tue, 2011-09-20 at 16:24 +0200, Johannes Weiner wrote: > On Tue, Sep 20, 2011 at 04:20:31PM +0200, Johannes Weiner wrote: > > On Mon, Sep 19, 2011 at 05:20:43PM -0400, Steven Rostedt wrote: > > > From: Steven Rostedt <srostedt@redhat.com> > > > > > > The code in memcg_check_events() calls this_cpu_read() on > > > different variables without disabling preemption, and can cause > > > the calculations to be done from two different CPU variables. > > > > > > Disable preemption throughout the check to keep apples and oranges > > > from becoming a mixed drink. > > > > Makes sense, thanks! > > > > Since the atomic versions are no longer required with preemption > > disabled explicitely, could you also make the this_cpu ops in > > __memcg_event_check and __mem_cgroup_target_update non-atomic in the > > same go?
Although this patch set is RFC, I probably should go ahead and push this one forward, as it looks to be a real bug.
> > Sorry, shouldn't be on you, can you fold this in?
Sure, thanks!
-- Steve
> > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org> > --- > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index b76011a..9d4ba65 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -683,8 +683,8 @@ static bool __memcg_event_check(struct mem_cgroup *memcg, int target) > { > unsigned long val, next; > > - val = this_cpu_read(memcg->stat->events[MEM_CGROUP_EVENTS_COUNT]); > - next = this_cpu_read(memcg->stat->targets[target]); > + val = __this_cpu_read(memcg->stat->events[MEM_CGROUP_EVENTS_COUNT]); > + next = __this_cpu_read(memcg->stat->targets[target]); > /* from time_after() in jiffies.h */ > return ((long)next - (long)val < 0); > } > @@ -693,7 +693,7 @@ static void __mem_cgroup_target_update(struct mem_cgroup *memcg, int target) > { > unsigned long val, next; > > - val = this_cpu_read(memcg->stat->events[MEM_CGROUP_EVENTS_COUNT]); > + val = __this_cpu_read(memcg->stat->events[MEM_CGROUP_EVENTS_COUNT]); > > switch (target) { > case MEM_CGROUP_TARGET_THRESH: > @@ -709,7 +709,7 @@ static void __mem_cgroup_target_update(struct mem_cgroup *memcg, int target) > return; > } > > - this_cpu_write(memcg->stat->targets[target], next); > + __this_cpu_write(memcg->stat->targets[target], next); > } > > /*
| |