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 A2CA0C25B06 for ; Sun, 14 Aug 2022 15:07:54 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3A6386B0073; Sun, 14 Aug 2022 11:07:54 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3561F8D0001; Sun, 14 Aug 2022 11:07:54 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 21D9C6B0075; Sun, 14 Aug 2022 11:07:54 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 0EB956B0073 for ; Sun, 14 Aug 2022 11:07:54 -0400 (EDT) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id AC721AAF55 for ; Sun, 14 Aug 2022 15:07:53 +0000 (UTC) X-FDA: 79798528026.14.4024BB9 Received: from mail-pf1-f175.google.com (mail-pf1-f175.google.com [209.85.210.175]) by imf27.hostedemail.com (Postfix) with ESMTP id 50090401BC for ; Sun, 14 Aug 2022 15:07:53 +0000 (UTC) Received: by mail-pf1-f175.google.com with SMTP id k14so4908260pfh.0 for ; Sun, 14 Aug 2022 08:07:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc; bh=jcOfyY3rbjfU9BLS27mCS6B6DeTsKt/CKZatx5u5NeY=; b=g4xkGaxSZlIDFYI5+HoHclUGV7OtpUCqFcR9/JTsj8u36VeZZoJuZBUV4GXh1nEKU5 N+GBFyTnnRREu5SmQtN/ZZVFfUl7KMFGuWKQKEnC+pWh50YEvUUzcpIUix/FMiriCf0j ZY466uwhyv7d16k61N2oHiXRismmVpzxb2mQJk2Gao9/1k+tYrzxrZ5uE7wL4fYLwOAE QIEJRHR3S6Ms4yBZKE6v/AbQ9pZcXoKSMSHjfTcbeHcpfUF62j+maWKZQlz+QE3Oe3Vi Z0vaQInhxMCIXnvhp9QSJDHnCOdTemU7qhcXnsWvUcnBqJwKDEFx3pLix6q7Gba9Aago QxEA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc; bh=jcOfyY3rbjfU9BLS27mCS6B6DeTsKt/CKZatx5u5NeY=; b=JTIK0c3lVvGN8TDjEQ9YIAwaY/al8Fm4CghzOU/dQ23dZpyksWMLnj2NhjiXh8iqI3 ClZZ0D1RlDZ9W5Vzc/8xq5sK+0q/Tkrc1VHoZATxEiKSdZ2sadZtuB9FLlVyNjbfKqMj 1h8NQkWHepjRTvGAxk2+o+Vg70lXWtbH83/kDTMvxOfMWj/G6Xhl2nJqJkfXeBK4vw6V uFJ91xiDtCleGNmU/IR1XF7+nLhhARoSQO6y4+C2KbVhaUopMbTpY+03BD0pJDifTnLf SPbcDPZy59lljN2it3T2F7U/19CgtfmjeUz96hm138YGOkT5ZNGud45V7RBN6TFSRFNM I9UQ== X-Gm-Message-State: ACgBeo3PInIVdfPLijnYWObSk4+BdB0ywc26Ft6AZyWhN8y8YfuuTCmj +2UVAjlJCb6vZt5XW2TCQsg= X-Google-Smtp-Source: AA6agR4aC3QBiN4WqXcuw8ypbpzsNqg8UXuZikqWY2gJttG72K62mi2CCiysmJnhyy6Hog+Pd2rUAQ== X-Received: by 2002:a63:e241:0:b0:41b:b374:caf8 with SMTP id y1-20020a63e241000000b0041bb374caf8mr10762109pgj.310.1660489672088; Sun, 14 Aug 2022 08:07:52 -0700 (PDT) Received: from hyeyoo ([114.29.91.56]) by smtp.gmail.com with ESMTPSA id mg18-20020a17090b371200b001f200eabc65sm3232672pjb.41.2022.08.14.08.07.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 14 Aug 2022 08:07:50 -0700 (PDT) Date: Mon, 15 Aug 2022 00:07:45 +0900 From: Hyeonggon Yoo <42.hyeyoo@gmail.com> To: haoxin 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 Subject: Re: [PATCH V2 1/1] mm/slub: release kobject if kobject_init_and_add failed in sysfs_slab_add Message-ID: References: <20220811071844.74020-1-xhao@linux.alibaba.com> <20220811071844.74020-2-xhao@linux.alibaba.com> <6af16141-1f9d-d74c-1a76-c752417a4283@linux.alibaba.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <6af16141-1f9d-d74c-1a76-c752417a4283@linux.alibaba.com> ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1660489673; 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:dkim-signature; bh=jcOfyY3rbjfU9BLS27mCS6B6DeTsKt/CKZatx5u5NeY=; b=Imc1lsJDFsI2+LR/48EXuBdX0FpLgIVDiiDsEO9KAJa8LM95ID2kQQ/sHACEC2xIKI2tHM JKpwKnmBRipLJeqPSa3XJ7ffZ/N5Nsjge4aR6XmAfGpx79onqxCSDC+hglfYKq3I4yHkVO 2vQF/morzPj3eedW+SLuJvdaUtZ5kDY= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=g4xkGaxS; spf=pass (imf27.hostedemail.com: domain of 42.hyeyoo@gmail.com designates 209.85.210.175 as permitted sender) smtp.mailfrom=42.hyeyoo@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1660489673; a=rsa-sha256; cv=none; b=c7F7v+T4Z0mlqEWFdZ4U3is1mMul07FkPWwYAiD2W459ESvpaQoH9Xd6lrWrlCsLxhoOCC saZ0MXrluvoOQxNurtn31eRUtAq9h2bYN8Mw0KYt6NFArIxSAwWvuTM2FSMrT2J9USUxyT LNnWtgmoFi80helk5rL67WTi18mB/BA= X-Stat-Signature: g3xfrptxr8k7he3jucgto8h8rtsf3xuc X-Rspamd-Queue-Id: 50090401BC Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=g4xkGaxS; spf=pass (imf27.hostedemail.com: domain of 42.hyeyoo@gmail.com designates 209.85.210.175 as permitted sender) smtp.mailfrom=42.hyeyoo@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-Rspam-User: X-Rspamd-Server: rspam09 X-HE-Tag: 1660489673-558826 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: On Sun, Aug 14, 2022 at 09:48:54PM +0800, haoxin wrote: > > 在 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. That is in fact not a memory leak. some early caches are made when sysfs subsystem is not up, so it cannot call sysfs_slab_add(). So the solution is to call sysfs_slab_add() in late boot process by using initcall mechanism. One difference is that early caches does not fail to create because it failed to register sysfs group - because you do it later. Then, if kernel succeeded to create caches and then failed to register sysfs group later, what should it do? release resources that are being used actively? or just shutdown the system? No, it just normally uses caches but just ignore the failure. Because resources of caches are being used normally, it is not a *leak*. Thanks! > > 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 > > > -- Thanks, Hyeonggon