From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id D3151C25B06 for ; Sun, 14 Aug 2022 13:49:05 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2052D8D0002; Sun, 14 Aug 2022 09:49:05 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 1B5006B0074; Sun, 14 Aug 2022 09:49:05 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0A3A98D0002; Sun, 14 Aug 2022 09:49:05 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id EE4EE6B0073 for ; Sun, 14 Aug 2022 09:49:04 -0400 (EDT) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id BB5DAC02D6 for ; Sun, 14 Aug 2022 13:49:04 +0000 (UTC) X-FDA: 79798329408.14.9D5E5B7 Received: from out199-7.us.a.mail.aliyun.com (out199-7.us.a.mail.aliyun.com [47.90.199.7]) by imf29.hostedemail.com (Postfix) with ESMTP id 53F3F12008B for ; Sun, 14 Aug 2022 13:49:00 +0000 (UTC) X-Alimail-AntiSpam:AC=PASS;BC=-1|-1;BR=01201311R211e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=ay29a033018045176;MF=xhao@linux.alibaba.com;NM=1;PH=DS;RN=10;SR=0;TI=SMTPD_---0VM9mdW6_1660484934; Received: from 192.168.1.6(mailfrom:xhao@linux.alibaba.com fp:SMTPD_---0VM9mdW6_1660484934) by smtp.aliyun-inc.com; Sun, 14 Aug 2022 21:48:56 +0800 Message-ID: <6af16141-1f9d-d74c-1a76-c752417a4283@linux.alibaba.com> Date: Sun, 14 Aug 2022 21:48:54 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.1.0 Subject: Re: [PATCH V2 1/1] mm/slub: release kobject if kobject_init_and_add failed in sysfs_slab_add To: Hyeonggon Yoo <42.hyeyoo@gmail.com> Cc: cl@linux.com, penberg@kernel.org, rientjes@google.com, iamjoonsoo.kim@lge.com, akpm@linux-foundation.org, vbabka@suse.cz, roman.gushchin@linux.dev, linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20220811071844.74020-1-xhao@linux.alibaba.com> <20220811071844.74020-2-xhao@linux.alibaba.com> From: haoxin In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1660484942; a=rsa-sha256; cv=none; b=PpSDbmuw3nSAv7Vg68kM5qQSeCOtBM4MxuyE2pXrRoCQ64osmEblY9ijCjEMBjPR0LgPYc Ph2b96b1FGJn93jeZwggnqe6Hu4EUq1pEk7ytwrmnvpzCtrbfjSKkdR1+zIXZ7kMX92ha8 hSIb5Cg8/EGabZekalk4eGY0vTYUUZs= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=alibaba.com; spf=pass (imf29.hostedemail.com: domain of xhao@linux.alibaba.com designates 47.90.199.7 as permitted sender) smtp.mailfrom=xhao@linux.alibaba.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1660484942; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=2njbXYh7BVPf2Hd7PYD34WHxi0BRUV2JBAGZtZqf170=; b=XruV/9s2Q67i3nSBFNzltsbmZq3XHfqai5f+xLJGDh1TqfgVrTs4ir06OHTjz7dtzjuH/o yt4+9dN+nB/vSdCVO9WfHYIYfgSivVBVDS6a5un1fDyPRaA4tDHqn+FGgy2THcUEH56wpU yRVml2UhnZ7aupNKUbLuFoTzt9cui5A= X-Rspam-User: X-Stat-Signature: a9jwfthkegxbkurp15kt51o848pi1cb5 X-Rspamd-Queue-Id: 53F3F12008B Authentication-Results: imf29.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=alibaba.com; spf=pass (imf29.hostedemail.com: domain of xhao@linux.alibaba.com designates 47.90.199.7 as permitted sender) smtp.mailfrom=xhao@linux.alibaba.com X-Rspamd-Server: rspam01 X-HE-Tag: 1660484940-728161 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: 在 2022/8/14 下午4:05, Hyeonggon Yoo 写道: > On Thu, Aug 11, 2022 at 03:18:44PM +0800, Xin Hao wrote: >> In kobject_init_and_add() function, the refcount is setted by calling >> kobject_init() function, regardless of whether the return value is zero >> or not, therefore, we must call kobject_del(&s->kobj) to prevent memory >> of s->kobj is leaked. > TL;DR: IIUC current code works just fine > > After thinking more, I don't think the memory leak you said exist. > The space for s->kobj is freed in create_cache() when __kmem_cache_create() failed. Yes, Agree what you explain,  but  in slab_sysfs_init() function, it also call slab_sysfs_add() and  there no other function to release it,  so i think the memory leak still exist. > The situation here is: > > create_cache() { > s = kmem_cache_alloc(kmem_cache, GFP_KERNEL) > err = __kmem_cache_create() > if (err) > goto out_free_cache; > > out_free_cache: > kmem_cache_free(s) // s is freed here (including its kobject) > [...] > } > > __kmem_cache_create() { > [...] > > err = sysfs_slab_add(); > if (err) { > __kmem_cache_release(s); > return err; > } > } > > The primary goal of kobject_put() is to call release() function > of kobj_type (when reference becomes zero), which is kmem_cache_release(). > > kmem_cache_release() { > __kmem_cache_release(s) > kfree_const(s->name) > kmem_cache_free(s) > } > > But when slab_sysfs_add() failed, __kmem_cache_release() and > create_cache() releases resources related to the cache. > (Also its name is freed in kmem_cache_create_usercopy().) > > So IIUC current code works just fine! > >> Signed-off-by: Xin Hao >> --- >> mm/slub.c | 7 +++---- >> 1 file changed, 3 insertions(+), 4 deletions(-) >> >> diff --git a/mm/slub.c b/mm/slub.c >> index b1281b8654bd..940a3f52e07c 100644 >> --- a/mm/slub.c >> +++ b/mm/slub.c >> @@ -5981,19 +5981,18 @@ static int sysfs_slab_add(struct kmem_cache *s) >> >> err = sysfs_create_group(&s->kobj, &slab_attr_group); >> if (err) >> - goto out_del_kobj; >> + goto out; >> >> if (!unmergeable) { >> /* Setup first alias */ >> sysfs_slab_alias(s, s->name); >> } >> + return err; >> out: >> if (!unmergeable) >> kfree(name); >> + kobject_put(&s->kobj); >> return err; >> -out_del_kobj: >> - kobject_del(&s->kobj); > So related resources are released in create_cache(), instead of by > calling kobject_put(). > > But kobject_del() is still needed because it should unlink kobject > hierarchy when kobject_add() succeeded but sysfs_create_group() failed! > >> - goto out; >> } >> >> void sysfs_slab_unlink(struct kmem_cache *s) >> -- >> 2.31.0 >>