On Thu, Mar 20, 2014 at 09:40:20PM -0700, Hugh Dickins wrote:
> On Thu, 20 Mar 2014, Greg KH wrote:
> > On Wed, Mar 19, 2014 at 10:59:27PM -0700, Hugh Dickins wrote:
> > > On Wed, 19 Mar 2014, [email protected] wrote:
> > > > 
> > > > The patch below does not apply to the 3.13-stable tree.
> > > > If someone wants it applied there, or to any other stable or longterm
> > > > tree, then please email the backport, including the original git commit
> > > > id to <[email protected]>.
> > > > 
> > > > thanks,
> > > > 
> > > > greg k-h
> > > > 
> > > ------------------ version for 3.13.7 ------------------
> > 
> > Thanks, now applied.
> 
> Thanks for including this in the 3.13.7-stable review series, Greg.
> But I notice there wasn't one in the 3.10.34-stable review series:
> I now think that I should have interpreted your FAILED on 3.13 mail
> as a prompt to send equivalents for the earlier releases too, sorry.
> 
> The version for Jiri's 3.12.15 would be the same as that for 3.13.7.
> 
> But the version for 3.10.34 (or perhaps now 3.10.35) is this below.
> Yes, more differences, and the old mem_cgroup_reparent_charges line
> is intentionally left in for 3.10 whereas it was removed for 3.12+:
> that's because the css/cgroup iterator changed in between, it used
> not to supply the root of the subtree, but nowadays it does.
> 
> Thanks,
> Hugh
> 
> ------------------ version for 3.10.34 ------------------
> 
> From 4fb1a86fb5e4209a7d4426d4e586c58e9edc74ac Mon Sep 17 00:00:00 2001
> From: Filipe Brandenburger <[email protected]>
> Date: Mon, 3 Mar 2014 15:38:25 -0800
> Subject: [PATCH] memcg: reparent charges of children before processing parent
> 
> Sometimes the cleanup after memcg hierarchy testing gets stuck in
> mem_cgroup_reparent_charges(), unable to bring non-kmem usage down to 0.
> 
> There may turn out to be several causes, but a major cause is this: the
> workitem to offline parent can get run before workitem to offline child;
> parent's mem_cgroup_reparent_charges() circles around waiting for the
> child's pages to be reparented to its lrus, but it's holding
> cgroup_mutex which prevents the child from reaching its
> mem_cgroup_reparent_charges().
> 
> Further testing showed that an ordered workqueue for cgroup_destroy_wq
> is not always good enough: percpu_ref_kill_and_confirm's call_rcu_sched
> stage on the way can mess up the order before reaching the workqueue.
> 
> Instead, when offlining a memcg, call mem_cgroup_reparent_charges() on
> all its children (and grandchildren, in the correct order) to have their
> charges reparented first.
> 
> Fixes: e5fca243abae ("cgroup: use a dedicated workqueue for cgroup 
> destruction")
> Signed-off-by: Filipe Brandenburger <[email protected]>
> Signed-off-by: Hugh Dickins <[email protected]>
> Reviewed-by: Tejun Heo <[email protected]>
> Acked-by: Michal Hocko <[email protected]>
> Cc: Johannes Weiner <[email protected]>
> Cc: <[email protected]>  [v3.10+]
> Signed-off-by: Andrew Morton <[email protected]>
> Signed-off-by: Linus Torvalds <[email protected]>
> 
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -6326,9 +6326,23 @@ static void mem_cgroup_invalidate_reclai
>  static void mem_cgroup_css_offline(struct cgroup *cont)
>  {
>       struct mem_cgroup *memcg = mem_cgroup_from_cont(cont);
> +     struct cgroup *iter;
>  
>       mem_cgroup_invalidate_reclaim_iterators(memcg);
> +
> +     /*
> +      * This requires that offlining is serialized.  Right now that is
> +      * guaranteed because css_killed_work_fn() holds the cgroup_mutex.
> +      */
> +     rcu_read_lock();
> +     cgroup_for_each_descendant_post(iter, cont) {
> +             rcu_read_unlock();
> +             mem_cgroup_reparent_charges(mem_cgroup_from_cont(iter));
> +             rcu_read_lock();
> +     }
> +     rcu_read_unlock();
>       mem_cgroup_reparent_charges(memcg);

Is this correct? ^^^

I may be missing something, but I believe this call to
mem_cgroup_reparent_charges() should be dropped (as in the original commit
and in your 3.13 backport).

Cheers,
--
Luís

> +
>       mem_cgroup_destroy_all_caches(memcg);
>  }
>  
> --
> To unsubscribe from this list: send the line "unsubscribe stable" in
> the body of a message to [email protected]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to