On Fri 22-07-11 09:27:59, Daisuke Nishimura wrote:
> On Thu, 21 Jul 2011 14:42:23 +0200
> Michal Hocko <mhocko@xxxxxxx> wrote:
>
> > On Thu 21-07-11 13:47:04, Michal Hocko wrote:
> > > On Thu 21-07-11 19:30:51, KAMEZAWA Hiroyuki wrote:
> > > > On Thu, 21 Jul 2011 09:58:24 +0200
> > > > Michal Hocko <mhocko@xxxxxxx> wrote:
> > [...]
> > > > > --- a/mm/memcontrol.c
> > > > > +++ b/mm/memcontrol.c
> > > > > @@ -2166,7 +2165,8 @@ static void drain_all_stock(struct mem_cgroup *root_mem, bool sync)
> > > > >
> > > > > for_each_online_cpu(cpu) {
> > > > > struct memcg_stock_pcp *stock = &per_cpu(memcg_stock, cpu);
> > > > > - if (test_bit(FLUSHING_CACHED_CHARGE, &stock->flags))
> > > > > + if (root_mem == stock->cached &&
> > > > > + test_bit(FLUSHING_CACHED_CHARGE, &stock->flags))
> > > > > flush_work(&stock->work);
> > > >
> > > > Doesn't this new check handle hierarchy ?
> > > > css_is_ancestor() will be required if you do this check.
> > >
> > > Yes you are right. Will fix it. I will add a helper for the check.
> >
> > Here is the patch with the helper. The above will then read
> > if (mem_cgroup_same_or_subtree(root_mem, stock->cached))
> >
> I welcome this new helper function, but it can be used in
> memcg_oom_wake_function() and mem_cgroup_under_move() too, can't it ?
Sure. Incremental patch (I will fold it into the one above):
---
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 8dbb9d6..64569c7 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1416,10 +1416,9 @@ static bool mem_cgroup_under_move(struct mem_cgroup *mem)
to = mc.to;
if (!from)
goto unlock;
- if (from == mem || to == mem
- || (mem->use_hierarchy && css_is_ancestor(&from->css, &mem->css))
- || (mem->use_hierarchy && css_is_ancestor(&to->css, &mem->css)))
- ret = true;
+
+ ret = mem_cgroup_same_or_subtree(mem, from)
+ || mem_cgroup_same_or_subtree(mem, to);
unlock:
spin_unlock(&mc.lock);
return ret;
@@ -1906,25 +1905,20 @@ struct oom_wait_info {
static int memcg_oom_wake_function(wait_queue_t *wait,
unsigned mode, int sync, void *arg)
{
- struct mem_cgroup *wake_mem = (struct mem_cgroup *)arg;
+ struct mem_cgroup *wake_mem = (struct mem_cgroup *)arg,
+ *oom_wait_mem;
struct oom_wait_info *oom_wait_info;
oom_wait_info = container_of(wait, struct oom_wait_info, wait);
+ oom_wait_mem = oom_wait_info->mem;
- if (oom_wait_info->mem == wake_mem)
- goto wakeup;
- /* if no hierarchy, no match */
- if (!oom_wait_info->mem->use_hierarchy || !wake_mem->use_hierarchy)
- return 0;
/*
* Both of oom_wait_info->mem and wake_mem are stable under us.
* Then we can use css_is_ancestor without taking care of RCU.
*/
- if (!css_is_ancestor(&oom_wait_info->mem->css, &wake_mem->css) &&
- !css_is_ancestor(&wake_mem->css, &oom_wait_info->mem->css))
+ if (!mem_cgroup_same_or_subtree(oom_wait_mem, wake_mem)
+ && !mem_cgroup_same_or_subtree(wake_mem, oom_wait_mem))
return 0;
-
-wakeup:
return autoremove_wake_function(wait, mode, sync, arg);
}
--
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>