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 4FA0FC433FE for ; Fri, 30 Sep 2022 11:25:58 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 803118D0002; Fri, 30 Sep 2022 07:25:57 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7B3048D0001; Fri, 30 Sep 2022 07:25:57 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6535D8D0002; Fri, 30 Sep 2022 07:25:57 -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 535B18D0001 for ; Fri, 30 Sep 2022 07:25:57 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 20F151407B3 for ; Fri, 30 Sep 2022 11:25:57 +0000 (UTC) X-FDA: 79968522354.24.CC40F4D Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by imf18.hostedemail.com (Postfix) with ESMTP id 4E6E61C000B for ; Fri, 30 Sep 2022 11:25:56 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id D7D101F8E6; Fri, 30 Sep 2022 11:25:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1664537154; h=from:from:reply-to: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=ngrX+6DubBpkNx5gzv1ZhIHn6iktJ17lFM9ULCLCMzo=; b=HWGaPVmZzbydhpEJk2sDAvgqv+LZUhlQJfn/FAnIzVlMyGcJ0mLDd80KbDX8sQg6z9qC4Y S2pP0fhv5wYfmenQoVHvvONR+piDSNn1bTV/Fzm9dR78LwHVtCEZj9aWER0+0Y91mc5lOn 9Awrus8EE9c/XSoBmWSlwfqRmmy2anc= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1664537154; h=from:from:reply-to: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=ngrX+6DubBpkNx5gzv1ZhIHn6iktJ17lFM9ULCLCMzo=; b=Lcv09zsxozpaGHxxPF919cY7e4VVxICltbrP+oDQlDxry+IzdgU5wqSmTi6eMv3xRTa+qN wyhtiL2PnKXdyrAQ== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id B35D713776; Fri, 30 Sep 2022 11:25:54 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id 4YT4KkLSNmOGDQAAMHmgww (envelope-from ); Fri, 30 Sep 2022 11:25:54 +0000 Message-ID: <13417330-2a81-e775-8875-fb31a5d5e078@suse.cz> Date: Fri, 30 Sep 2022 13:25:54 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.3.0 Subject: Re: [PATCH -next] mm/slub: fix a slab missed to be freed problem Content-Language: en-US To: Feng Tang , Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Roman Gushchin , Hyeonggon Yoo <42.hyeyoo@gmail.com> Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20220930100730.250248-1-feng.tang@intel.com> From: Vlastimil Babka In-Reply-To: <20220930100730.250248-1-feng.tang@intel.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1664537156; a=rsa-sha256; cv=none; b=KlHTTNkTsSs3Z+qpstzXILXlodvdEZaB88hhoRKhmzQglbfnIkRGsA/BT2AZys+f/sXwtt xBzECU6wH+Z5hKu+XbDblKKDXXhHcmdlO13xZE27HNkRCc2Myivgox30GvMiNF7oZJPVHT PPORDhpVFXMwDC9DObZZMoiu62uLxuA= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=HWGaPVmZ; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=Lcv09zsx; dmarc=none; spf=pass (imf18.hostedemail.com: domain of vbabka@suse.cz designates 195.135.220.29 as permitted sender) smtp.mailfrom=vbabka@suse.cz ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1664537156; 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=ngrX+6DubBpkNx5gzv1ZhIHn6iktJ17lFM9ULCLCMzo=; b=7IA0Ggr8fCf5xfKNDRQdNT1204FuvifRJ8lL0tjacBmD6JkOTGPPBpbdADpLUvhqcbCI04 bfhxv0wsLRoJviRRhV4Rj0ukPCJqyAQ9qlpba5XLPYVyr4e79QNkBSeY5Yf2/YKz7Sh3GZ UezxqDsYXJQYxWUfn1FehTXbhwNSj0g= X-Rspamd-Queue-Id: 4E6E61C000B X-Rspam-User: Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=HWGaPVmZ; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=Lcv09zsx; dmarc=none; spf=pass (imf18.hostedemail.com: domain of vbabka@suse.cz designates 195.135.220.29 as permitted sender) smtp.mailfrom=vbabka@suse.cz X-Rspamd-Server: rspam03 X-Stat-Signature: b4buhh8oi4obnnwmeqjtrhtyk8xjaryt X-HE-Tag: 1664537156-891378 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 9/30/22 12:07, Feng Tang wrote: > When enable kasan and kfence's in-kernel kunit test with slub_debug on, > it caught a problem (in linux-next tree): > > ------------[ cut here ]------------ > kmem_cache_destroy test: Slab cache still has objects when called from test_exit+0x1a/0x30 > WARNING: CPU: 3 PID: 240 at mm/slab_common.c:492 kmem_cache_destroy+0x16c/0x170 Assuming the warning was preceded by some kunit test failures? I don't see how leaving more empty slabs on free list than needed would cause this warning, the shutdown should just drop the empty slab. > Modules linked in: > CPU: 3 PID: 240 Comm: kunit_try_catch Tainted: G B N 6.0.0-rc7-next-20220929 #52 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014 > RIP: 0010:kmem_cache_destroy+0x16c/0x170 > Code: 41 5c 41 5d e9 a5 04 0b 00 c3 cc cc cc cc 48 8b 55 60 48 8b 4c 24 20 48 c7 c6 40 37 d2 82 48 c7 c7 e8 a0 33 83 e8 4e d7 14 01 <0f> 0b eb a7 41 56 41 89 d6 41 55 49 89 f5 41 54 49 89 fc 55 48 89 > RSP: 0000:ffff88800775fea0 EFLAGS: 00010282 > RAX: 0000000000000000 RBX: ffffffff83bdec48 RCX: 0000000000000000 > RDX: 0000000000000001 RSI: 1ffff11000eebf9e RDI: ffffed1000eebfc6 > RBP: ffff88804362fa00 R08: ffffffff81182e58 R09: ffff88800775fbdf > R10: ffffed1000eebf7b R11: 0000000000000001 R12: 000000008c800d00 > R13: ffff888005e78040 R14: 0000000000000000 R15: ffff888005cdfad0 > FS: 0000000000000000(0000) GS:ffff88807ed00000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 0000000000000000 CR3: 000000000360e001 CR4: 0000000000370ee0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > Call Trace: > > test_exit+0x1a/0x30 > kunit_try_run_case+0xad/0xc0 > kunit_generic_run_threadfn_adapter+0x26/0x50 > kthread+0x17b/0x1b0 > > It was biscted to commit c7323a5ad078 ("mm/slub: restrict sysfs > validation to debug caches and make it safe") > > The problem is inside free_debug_processing(), in one path, the slab > on partial list is missed to be freed when partial list is full. > > Signed-off-by: Feng Tang > --- > > Hi reviewers, > > Sorry for the late reporting, but it's curious that this problem didn't > show up in my earlier test (which caught some other problems). I think we can reuse the slab_free and don't need a new bool? diff --git a/mm/slub.c b/mm/slub.c index 5c3c31a154ba..a63953f649ed 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -2886,22 +2886,25 @@ static noinline void free_debug_processing( set_freepointer(s, tail, prior); slab->freelist = head; - /* Do we need to remove the slab from full or partial list? */ + /* + * If the slab is empty, and node's partial list is full, + * it should be discarded anyway no matter it's on full or + * partial list. + */ + if (slab->inuse == 0 && n->nr_partial >= s->min_partial) + slab_free = slab; + if (!prior) { + /* was on full list */ remove_full(s, n, slab); - } else if (slab->inuse == 0 && - n->nr_partial >= s->min_partial) { + if (!slab_free) { + add_partial(n, slab, DEACTIVATE_TO_TAIL); + stat(s, FREE_ADD_PARTIAL); + } + } else if (slab_free) { remove_partial(n, slab); stat(s, FREE_REMOVE_PARTIAL); } - - /* Do we need to discard the slab or add to partial list? */ - if (slab->inuse == 0 && n->nr_partial >= s->min_partial) { - slab_free = slab; - } else if (!prior) { - add_partial(n, slab, DEACTIVATE_TO_TAIL); - stat(s, FREE_ADD_PARTIAL); - } } if (slab_free) { > mm/slub.c | 28 +++++++++++++++++----------- > 1 file changed, 17 insertions(+), 11 deletions(-) > > diff --git a/mm/slub.c b/mm/slub.c > index 5c3c31a154ba..4c037bd0b22b 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -2880,28 +2880,34 @@ static noinline void free_debug_processing( > out: > if (checks_ok) { > void *prior = slab->freelist; > + bool slab_need_discard = false; > > /* Perform the actual freeing while we still hold the locks */ > slab->inuse -= cnt; > set_freepointer(s, tail, prior); > slab->freelist = head; > > - /* Do we need to remove the slab from full or partial list? */ > + /* > + * If the slab is empty, and node's partial list is full, > + * it should be discarded anyway no matter it's on full or > + * partial list. > + */ > + if (slab->inuse == 0 && n->nr_partial >= s->min_partial) { > + slab_need_discard = true; > + slab_free = slab; > + } > + > if (!prior) { > + /* was on full list */ > remove_full(s, n, slab); > - } else if (slab->inuse == 0 && > - n->nr_partial >= s->min_partial) { > + if (!slab_need_discard) { > + add_partial(n, slab, DEACTIVATE_TO_TAIL); > + stat(s, FREE_ADD_PARTIAL); > + } > + } else if (slab_need_discard) { > remove_partial(n, slab); > stat(s, FREE_REMOVE_PARTIAL); > } > - > - /* Do we need to discard the slab or add to partial list? */ > - if (slab->inuse == 0 && n->nr_partial >= s->min_partial) { > - slab_free = slab; > - } else if (!prior) { > - add_partial(n, slab, DEACTIVATE_TO_TAIL); > - stat(s, FREE_ADD_PARTIAL); > - } > } > > if (slab_free) {