From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1761867Ab3DCJs6 (ORCPT ); Wed, 3 Apr 2013 05:48:58 -0400 Received: from cantor2.suse.de ([195.135.220.15]:59572 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1761000Ab3DCJs4 (ORCPT ); Wed, 3 Apr 2013 05:48:56 -0400 Date: Wed, 3 Apr 2013 11:48:53 +0200 From: Michal Hocko To: linux-mm@kvack.org Cc: Li Zefan , Glauber Costa , Johannes Weiner , KAMEZAWA Hiroyuki , linux-kernel@vger.kernel.org, cgroups@vger.kernel.org Subject: Re: [PATCH 2/2] memcg, kmem: clean up reference count handling on the error path Message-ID: <20130403094853.GG16471@dhcp22.suse.cz> References: <20130403085056.GD14384@dhcp22.suse.cz> <1364979234-16427-1-git-send-email-mhocko@suse.cz> <1364979234-16427-2-git-send-email-mhocko@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1364979234-16427-2-git-send-email-mhocko@suse.cz> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed 03-04-13 10:53:54, Michal Hocko wrote: > mem_cgroup_css_online calls mem_cgroup_put if memcg_init_kmem > fails. This is not correct because only memcg_propagate_kmem takes an > additional reference while mem_cgroup_sockets_init is allowed to fail as > well (although no current implementation fails) but it doesn't take any > reference. This all suggests that it should be memcg_propagate_kmem that > should clean up after itself so this patch moves mem_cgroup_put over > there. > Unfortunately this is not that easy (as pointed out by Li Zefan) because > memcg_kmem_mark_dead marks the group dead (KMEM_ACCOUNTED_DEAD) if it > is marked active (KMEM_ACCOUNTED_ACTIVE) which is the case even if > memcg_propagate_kmem fails so the additional reference is dropped in > that case in kmem_cgroup_destroy which means that the reference would be > dropped two times. > > The easiest way then would be to simply remove mem_cgrroup_put from > mem_cgroup_css_online and rely on kmem_cgroup_destroy doing the right > thing. Forgot to mention that this one could be marked for stable for 3.8 > Signed-off-by: Li Zefan > Signed-off-by: Michal Hocko > --- > mm/memcontrol.c | 8 -------- > 1 file changed, 8 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 6de6d70..65b2850 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -6417,14 +6417,6 @@ mem_cgroup_css_online(struct cgroup *cont) > > error = memcg_init_kmem(memcg, &mem_cgroup_subsys); > mutex_unlock(&memcg_create_mutex); > - if (error) { > - /* > - * We call put now because our (and parent's) refcnts > - * are already in place. mem_cgroup_put() will internally > - * call __mem_cgroup_free, so return directly > - */ > - mem_cgroup_put(memcg); > - } > return error; > } > > -- > 1.7.10.4 > > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to majordomo@kvack.org. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Don't email: email@kvack.org -- Michal Hocko SUSE Labs