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 044C6C4332F for ; Mon, 3 Oct 2022 09:38:36 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4B1F46B0072; Mon, 3 Oct 2022 05:38:36 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 461206B0073; Mon, 3 Oct 2022 05:38:36 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 329968E0001; Mon, 3 Oct 2022 05:38:36 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 1F8A76B0072 for ; Mon, 3 Oct 2022 05:38:36 -0400 (EDT) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id BB0661C6104 for ; Mon, 3 Oct 2022 09:38:35 +0000 (UTC) X-FDA: 79979138190.14.A05A231 Received: from mail-lf1-f42.google.com (mail-lf1-f42.google.com [209.85.167.42]) by imf13.hostedemail.com (Postfix) with ESMTP id E11E42000F for ; Mon, 3 Oct 2022 09:38:34 +0000 (UTC) Received: by mail-lf1-f42.google.com with SMTP id g1so15719265lfu.12 for ; Mon, 03 Oct 2022 02:38:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rasmusvillemoes.dk; s=google; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date; bh=dANBLy9Pkuer3zrizbGdj6SpwUeQniCxpZBW0aJuHzs=; b=DPMHZ2ZH4R+ci5dPn2ER3rf1R/0i913FAkq4g2QEV20Ow729LeCDzDJ520/7ON4ZQv jm6k/23rFDUYem1ZLhtvJp01+PXmOwsK1KcA9cMI6EHGSLob2hW2RfaxKpjJ2ZU+3Uad EzBqKorUWQ4YCt4BbgIWjpfU/bGW2Xpu+RpWY= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date; bh=dANBLy9Pkuer3zrizbGdj6SpwUeQniCxpZBW0aJuHzs=; b=kuk5rWbWivthSyeJWswB8uy9T9m2e02WTGHk52v35i/Uy8V7nog37dYVrPVWDZREhv vpYhkfjG05j3kPOK2AOQFslmvTKLCkouCPdgWVU5eI89jplnfx2MieWm1YK04/eXENt5 ldZq5YnHI8pPZoeRCRsLb98/BCRotA8CQqW4y23nEmEKq6eXn7F40L7nQXH6EIJ0/HAl PiIbyhCzSlifuqse6aSHq7HUAfqHBlKMst0bPanIDrLPErxtnnj13rRbWf0eo9JLmvgq TXV8lEC42WUlGx6npe9OuC4fGSvQx+UtRscos5qaCYm0GNkktWBAgflEJgxh40H2p49I iiNQ== X-Gm-Message-State: ACrzQf1oT1n/MT35dCiIDcy2awynxNF+qv0JPndUKZJDKLLZMANDgBYX KQMtjNfz55Vz60nvSsHQRAMzgA== X-Google-Smtp-Source: AMsMyM7bUkSugzAM3erycF3eY16LBjwRvF21M0DYTgKbtTm5ZgRoygtOV/AOZG8pLP7Rgg2JXbRe+A== X-Received: by 2002:ac2:5a08:0:b0:4a2:2db2:2dfb with SMTP id q8-20020ac25a08000000b004a22db22dfbmr2630885lfn.638.1664789913072; Mon, 03 Oct 2022 02:38:33 -0700 (PDT) Received: from [172.21.2.224] ([87.54.42.112]) by smtp.gmail.com with ESMTPSA id l20-20020a2e99d4000000b0026dce212f24sm539601ljj.98.2022.10.03.02.38.31 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 03 Oct 2022 02:38:32 -0700 (PDT) Message-ID: Date: Mon, 3 Oct 2022 11:38:30 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [PATCH] mm: slub: remove dead and buggy code from sysfs_slab_add() Content-Language: en-US To: Hyeonggon Yoo <42.hyeyoo@gmail.com> Cc: Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Andrew Morton , Vlastimil Babka , Roman Gushchin , linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20220930084742.771804-1-linux@rasmusvillemoes.dk> From: Rasmus Villemoes In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1664789915; 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=dANBLy9Pkuer3zrizbGdj6SpwUeQniCxpZBW0aJuHzs=; b=M0lqIat655NkTgn/TjINMT3CncedVrE9o4Or1DtSkfWDk3UaNR9qjek2cZqGZ2kO3S/dD5 zRjTl3CqQ2IeWQLm82USKIaRoOjthM/cEUtif6znaznAkHn9YBdHj+6tFrMq5lusffxwtI ez+jYQlx3H7tDwl3Z+pcqHXUVKcsDTM= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=rasmusvillemoes.dk header.s=google header.b=DPMHZ2ZH; dmarc=none; spf=pass (imf13.hostedemail.com: domain of linux@rasmusvillemoes.dk designates 209.85.167.42 as permitted sender) smtp.mailfrom=linux@rasmusvillemoes.dk ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1664789915; a=rsa-sha256; cv=none; b=iN2Tr67ZPXkxHcC//1wXnsHwzybLUwdNcKnBb7ocwJ7X7RwxCgKdANlcnJt8+f9+0+T5jK ydcVQ7zOkJTq/h8Y1qFZvS+sp7qKfgOvNQVL7qDPy5AVac9Hj+3IET/6I7HpmYsi+6xivH qecyjRuMWAXZrCUcn4gbe1Z7/wbIG0I= X-Stat-Signature: fhjkt81coetydnqu14k6msp1qrsq4nxz X-Rspam-User: X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: E11E42000F Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=rasmusvillemoes.dk header.s=google header.b=DPMHZ2ZH; dmarc=none; spf=pass (imf13.hostedemail.com: domain of linux@rasmusvillemoes.dk designates 209.85.167.42 as permitted sender) smtp.mailfrom=linux@rasmusvillemoes.dk X-HE-Tag: 1664789914-772821 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 03/10/2022 09.02, Hyeonggon Yoo wrote: > On Fri, Sep 30, 2022 at 10:47:42AM +0200, Rasmus Villemoes wrote: >> The function sysfs_slab_add() has two callers: >> >> One is slab_sysfs_init(), which first initializes slab_kset, and only >> when that succeeds sets slab_state to FULL, and then proceeds to call >> sysfs_slab_add() for all previously created slabs. >> >> The other is __kmem_cache_create(), but only after a >> >> if (slab_state <= UP) >> return 0; >> >> check. >> >> So in other words, sysfs_slab_add() is never called without >> slab_kset (aka the return value of cache_kset()) being non-NULL. >> >> And this is just as well, because if we ever did take this path and >> called kobject_init(&s->kobj), and then later when called again from >> slab_sysfs_init() would end up calling kobject_init_and_add(), we >> would hit >> >> if (kobj->state_initialized) { >> /* do not error out as sometimes we can recover */ >> pr_err("kobject (%p): tried to init an initialized object, something is seriously wrong.\n", >> dump_stack(); >> } >> >> in kobject.c. >> >> Signed-off-by: Rasmus Villemoes >> --- >> mm/slub.c | 5 ----- >> 1 file changed, 5 deletions(-) >> >> diff --git a/mm/slub.c b/mm/slub.c >> index 4b98dff9be8e..04a7f75a7b1f 100644 >> --- a/mm/slub.c >> +++ b/mm/slub.c >> @@ -5937,11 +5937,6 @@ static int sysfs_slab_add(struct kmem_cache *s) >> struct kset *kset = cache_kset(s); >> int unmergeable = slab_unmergeable(s); >> >> - if (!kset) { >> - kobject_init(&s->kobj, &slab_ktype); >> - return 0; >> - } >> - >> if (!unmergeable && disable_higher_order_debug && >> (slub_debug & DEBUG_METADATA_FLAGS)) >> unmergeable = 1; >> -- >> 2.37.2 > > I assumed that it's hit when SLUB failed to initialize slab_kset in > slab_sysfs_init(). (Yeah, it is too unlikely, though....) No, it is not, because if the creation of slab_kset fails, slab_sysfs_init() returns early, and hence slab_state never transitions to FULL. I don't see anywhere else where slab_state could become FULL (of course in slab.c and slob.c, but those are not built when slub.c is), so I do believe my analysis in the commit log is correct. > And obviously it's a bug if sysfs_slab_add() is called early than > slab_sysfs_init(). Yes, and that's already what the existing slab_state check guards. Rasmus