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 962C3C3F6B0 for ; Sat, 13 Aug 2022 15:01:08 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7F1B28E0002; Sat, 13 Aug 2022 11:01:07 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7A2308E0001; Sat, 13 Aug 2022 11:01:07 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 61A958E0002; Sat, 13 Aug 2022 11:01:07 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 4B8168E0001 for ; Sat, 13 Aug 2022 11:01:07 -0400 (EDT) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 21AE2A0D76 for ; Sat, 13 Aug 2022 15:01:07 +0000 (UTC) X-FDA: 79794882174.06.D813956 Received: from mail-pg1-f179.google.com (mail-pg1-f179.google.com [209.85.215.179]) by imf25.hostedemail.com (Postfix) with ESMTP id 9830BA01A1 for ; Sat, 13 Aug 2022 15:01:06 +0000 (UTC) Received: by mail-pg1-f179.google.com with SMTP id 202so3080734pgc.8 for ; Sat, 13 Aug 2022 08:01:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc; bh=QiqCev7SjvOWKNNl+hDjUBDJkQvgTrLbkZw98Qqxk70=; b=ERPwgAquhldV9Lao9bt3DOyrwIEOHvpV8mjiwyMqzztYaV+Qs7fDZ9aLffZ+ffWj6B LD0oToVrrj1+879zUEzE2gvNyg0vkYBHCrQ3ohRL6H5Eh9c+AmjTwVZeoGOuzeHUIX4/ UidNqrwhKyeAn5hMG9kXmqFfta60uv62Q44DJ1lt3ynXGZa4Lk153zK5mir2cVvrOZRZ 84d5RiT30TGVFAfc92D7dKWSXnOpAEkq3gJs7x2pMtCzvgIgOh7yhR7u/cqewFAYioYv 7NDTpv2WaQKwVVzb2XxOVY4q7dNmBHYNRSFJRr2nhOuugNwYkAIsDIzbq+fZcEJlKn4C 9jpA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc; bh=QiqCev7SjvOWKNNl+hDjUBDJkQvgTrLbkZw98Qqxk70=; b=2z4i3kAtI7NMnqvOIHfI+2HmTTuS/DRqWXo4CrsYX73HElalgDzx/iv+oKbbRnrGUF s9jIUpZyCuUiQnEaz3MCrbkwNXEvYzFVCtGEssLuJcNjAbpC5G11Lgvuos4vcY3SmFze yrbp0az3Dzni5piFGlfFU9HRQTD0wzXvpVX1pbuw5o62XwRPwd3bYgoMNyReaKMOloLK U815FXGjyH+TOP++8TEXSjgquNvSsgXX9RcyndzJOQqeuVQKYe1wDdMyQdv5Hn7Mn/xy hKlS2doaZOx6BJM7laFN2EwuPluk0FAqDH9U+0f3NL+Ll8TlW1n2nANbLW7h4Hvn+sKt y4tQ== X-Gm-Message-State: ACgBeo3h2QFZUyJsbC9w1r88IeyDfsXh3s2MUq0UIdI/A7LNQ/YaRLrh o9ZT/w8j2Rx2MkqgsTjpF9Y= X-Google-Smtp-Source: AA6agR5olXFk9GY/Fk0KTm+LgbNHC4nyJYZJobVnKm6MiOqEFVE8HFrztfSEmVpGNCBc1epxFziCQA== X-Received: by 2002:a63:5a4d:0:b0:41b:7702:635f with SMTP id k13-20020a635a4d000000b0041b7702635fmr7249032pgm.111.1660402865394; Sat, 13 Aug 2022 08:01:05 -0700 (PDT) Received: from hyeyoo ([114.29.91.56]) by smtp.gmail.com with ESMTPSA id 6-20020a170902c10600b00172543d7cdcsm2623630pli.91.2022.08.13.08.01.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 13 Aug 2022 08:01:04 -0700 (PDT) Date: Sun, 14 Aug 2022 00:00:58 +0900 From: Hyeonggon Yoo <42.hyeyoo@gmail.com> To: Waiman Long Cc: Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Andrew Morton , Vlastimil Babka , Roman Gushchin , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v3] mm/slab_common: Deleting kobject in kmem_cache_destroy() without holding slab_mutex/cpu_hotplug_lock Message-ID: References: <20220812183033.346425-1-longman@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220812183033.346425-1-longman@redhat.com> ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1660402866; a=rsa-sha256; cv=none; b=dZyV80GtzhXpFjiVUkSO8ETgS0RwjGj3stssL4CN/FKSw2QnGzi5dfWR3MhV5KJl2BfOaM 7TV0OznInyB7yK/oosZifp01wvtIZ390jsCK8YjET9CarPEMaLPc5DdPFCbOzZGO34otMZ h/2qurty8dP8xFpd7Zs+0j/qfHBjj50= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1660402866; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=QiqCev7SjvOWKNNl+hDjUBDJkQvgTrLbkZw98Qqxk70=; b=2a+w2IEOGRIK0HoZIFxsm271eRdnaF3CCj24dRztCLTnvly1Cz+8ZPH6T8Ec9ULOmXmLD/ omLV6RhHAu3vw8fY9t77Btofwjo3knF6/9IpifHEABVTRHS3F8sebFTZSClnlXFEl3aEZV aUP5RjJoomZQhQUymXiuWzUzRYZ1gfk= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=ERPwgAqu; spf=pass (imf25.hostedemail.com: domain of 42.hyeyoo@gmail.com designates 209.85.215.179 as permitted sender) smtp.mailfrom=42.hyeyoo@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-Stat-Signature: pxjnrnukgixkmmu4cpc19z7ha1idnmqb X-Rspamd-Queue-Id: 9830BA01A1 Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=ERPwgAqu; spf=pass (imf25.hostedemail.com: domain of 42.hyeyoo@gmail.com designates 209.85.215.179 as permitted sender) smtp.mailfrom=42.hyeyoo@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-Rspam-User: X-Rspamd-Server: rspam04 X-HE-Tag: 1660402866-907555 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 Fri, Aug 12, 2022 at 02:30:33PM -0400, Waiman Long wrote: > A circular locking problem is reported by lockdep due to the following > circular locking dependency. > > +--> cpu_hotplug_lock --> slab_mutex --> kn->active --+ > | | > +-----------------------------------------------------+ > > The forward cpu_hotplug_lock ==> slab_mutex ==> kn->active dependency > happens in > > kmem_cache_destroy(): cpus_read_lock(); mutex_lock(&slab_mutex); > ==> sysfs_slab_unlink() > ==> kobject_del() > ==> kernfs_remove() > ==> __kernfs_remove() > ==> kernfs_drain(): rwsem_acquire(&kn->dep_map, ...); Maybe you mean this? /* but everyone should wait for draining */ wait_event(root->deactivate_waitq, atomic_read(&kn->active) == KN_DEACTIVATED_BIAS); > The backward kn->active ==> cpu_hotplug_lock dependency happens in > > kernfs_fop_write_iter(): kernfs_get_active(); > ==> slab_attr_store() > ==> cpu_partial_store() > ==> flush_all(): cpus_read_lock() > > One way to break this circular locking chain is to avoid holding > cpu_hotplug_lock and slab_mutex while deleting the kobject in > sysfs_slab_unlink() which should be equivalent to doing a write_lock > and write_unlock pair of the kn->active virtual lock. > > Since the kobject structures are not protected by slab_mutex or the > cpu_hotplug_lock, we can certainly release those locks before doing > the delete operation. > > Move sysfs_slab_unlink() and sysfs_slab_release() to the newly > created kmem_cache_release() and call it outside the slab_mutex & > cpu_hotplug_lock critical sections. There will be a slight delay > in the deletion of sysfs files if kmem_cache_release() is called > indirectly from a work function. > > Signed-off-by: Waiman Long > --- > > [v3] Move sysfs_slab_unlink() out to kmem_cache_release() and move > schedule_work() back right after list_add_tail(). > > mm/slab_common.c | 45 +++++++++++++++++++++++++++++---------------- > 1 file changed, 29 insertions(+), 16 deletions(-) > > diff --git a/mm/slab_common.c b/mm/slab_common.c > index 17996649cfe3..07b948288f84 100644 > --- a/mm/slab_common.c > +++ b/mm/slab_common.c > @@ -392,6 +392,28 @@ kmem_cache_create(const char *name, unsigned int size, unsigned int align, > } > EXPORT_SYMBOL(kmem_cache_create); > > +#ifdef SLAB_SUPPORTS_SYSFS > +/* > + * For a given kmem_cache, kmem_cache_destroy() should only be called > + * once or there will be a use-after-free problem. The actual deletion > + * and release of the kobject does not need slab_mutex or cpu_hotplug_lock > + * protection. So they are now done without holding those locks. > + * > + * Note that there will be a slight delay in the deletion of sysfs files > + * if kmem_cache_release() is called indrectly from a work function. > + */ > +static void kmem_cache_release(struct kmem_cache *s) > +{ > + sysfs_slab_unlink(s); > + sysfs_slab_release(s); > +} > +#else > +static void kmem_cache_release(struct kmem_cache *s) > +{ > + slab_kmem_cache_release(s); > +} > +#endif > + > static void slab_caches_to_rcu_destroy_workfn(struct work_struct *work) > { > LIST_HEAD(to_destroy); > @@ -418,11 +440,7 @@ static void slab_caches_to_rcu_destroy_workfn(struct work_struct *work) > list_for_each_entry_safe(s, s2, &to_destroy, list) { > debugfs_slab_release(s); > kfence_shutdown_cache(s); > -#ifdef SLAB_SUPPORTS_SYSFS > - sysfs_slab_release(s); > -#else > - slab_kmem_cache_release(s); > -#endif > + kmem_cache_release(s); > } > } > > @@ -437,20 +455,11 @@ static int shutdown_cache(struct kmem_cache *s) > list_del(&s->list); > > if (s->flags & SLAB_TYPESAFE_BY_RCU) { > -#ifdef SLAB_SUPPORTS_SYSFS > - sysfs_slab_unlink(s); > -#endif > list_add_tail(&s->list, &slab_caches_to_rcu_destroy); > schedule_work(&slab_caches_to_rcu_destroy_work); > } else { > kfence_shutdown_cache(s); > debugfs_slab_release(s); > -#ifdef SLAB_SUPPORTS_SYSFS > - sysfs_slab_unlink(s); > - sysfs_slab_release(s); > -#else > - slab_kmem_cache_release(s); > -#endif > } > > return 0; > @@ -465,14 +474,16 @@ void slab_kmem_cache_release(struct kmem_cache *s) > > void kmem_cache_destroy(struct kmem_cache *s) > { > + int refcnt; > + > if (unlikely(!s) || !kasan_check_byte(s)) > return; > > cpus_read_lock(); > mutex_lock(&slab_mutex); > > - s->refcount--; > - if (s->refcount) > + refcnt = --s->refcount; > + if (refcnt) > goto out_unlock; > > WARN(shutdown_cache(s), > @@ -481,6 +492,8 @@ void kmem_cache_destroy(struct kmem_cache *s) > out_unlock: > mutex_unlock(&slab_mutex); > cpus_read_unlock(); > + if (!refcnt && !(s->flags & SLAB_TYPESAFE_BY_RCU)) > + kmem_cache_release(s); > } > EXPORT_SYMBOL(kmem_cache_destroy); > > -- > 2.31.1 > little bit complicated but looks good to me. Thank you for fixing this. Reviewed-by: Hyeonggon Yoo <42.hyeyoo@gmail.com> -- Thanks, Hyeonggon