linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Vlastimil Babka <vbabka@suse.cz>
To: "Paul E. McKenney" <paulmck@kernel.org>,
	 Joel Fernandes <joel@joelfernandes.org>,
	 Josh Triplett <josh@joshtriplett.org>,
	Boqun Feng <boqun.feng@gmail.com>,
	 Christoph Lameter <cl@linux.com>,
	David Rientjes <rientjes@google.com>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	 Lai Jiangshan <jiangshanlai@gmail.com>,
	Zqiang <qiang.zhang1211@gmail.com>,
	 Julia Lawall <Julia.Lawall@inria.fr>,
	Jakub Kicinski <kuba@kernel.org>,
	 "Jason A. Donenfeld" <Jason@zx2c4.com>,
	 "Uladzislau Rezki (Sony)" <urezki@gmail.com>,
	 Andrew Morton <akpm@linux-foundation.org>,
	 Roman Gushchin <roman.gushchin@linux.dev>,
	 Hyeonggon Yoo <42.hyeyoo@gmail.com>,
	linux-mm@kvack.org,  linux-kernel@vger.kernel.org,
	rcu@vger.kernel.org,  Vlastimil Babka <vbabka@suse.cz>
Subject: [PATCH RFC 3/6] mm, slab: unlink sysfs and debugfs immediately
Date: Mon, 15 Jul 2024 22:29:29 +0200	[thread overview]
Message-ID: <20240715-b4-slab-kfree_rcu-destroy-v1-3-46b2984c2205@suse.cz> (raw)
In-Reply-To: <20240715-b4-slab-kfree_rcu-destroy-v1-0-46b2984c2205@suse.cz>

kmem_cache_destroy() includes removing the associated sysfs and debugfs
directories. Currently this might not happen immediately when:

- the cache is SLAB_TYPESAFE_BY_RCU and the cleanup is delayed,
  including the directores removal
- __kmem_cache_shutdown() fails due to outstanding objects - the
  directories remain indefinitely

When a cache is recreated with the same name, such as due to module
unload followed by a load, the directories will fail to be recreated for
the new instance of the cache due to the old directories being present.

We also want to add another possibility of delayed cleanup due to
kfree_rcu() in flight so let's fix this first and have the directories
removed immediately in kmem_cache_destroy() and regardless of
__kmem_cache_shutdown() success.

This should not make debugging harder if __kmem_cache_shutdown() fails,
because a detailed report of outstanding objects is printed into dmesg
already due to the failure.

Note the record in /proc/slabinfo will remain until the cleanup is
finished (or indefinitely if __kmem_cache_shutdown() fails) but that
does not prevent a new record to be added for a new cache instance.

Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
 mm/slab_common.c | 65 ++++++++++++++++++++++++++++----------------------------
 1 file changed, 32 insertions(+), 33 deletions(-)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index 3ba205bda95d..2eef5ad37fa7 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -492,14 +492,10 @@ EXPORT_SYMBOL(kmem_buckets_create);
  * 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)
 {
 	if (slab_state >= FULL) {
-		sysfs_slab_unlink(s);
 		sysfs_slab_release(s);
 	} else {
 		slab_kmem_cache_release(s);
@@ -536,33 +532,11 @@ static void slab_caches_to_rcu_destroy_workfn(struct work_struct *work)
 	rcu_barrier();
 
 	list_for_each_entry_safe(s, s2, &to_destroy, list) {
-		debugfs_slab_release(s);
 		kfence_shutdown_cache(s);
 		kmem_cache_release(s);
 	}
 }
 
-static int shutdown_cache(struct kmem_cache *s)
-{
-	/* free asan quarantined objects */
-	kasan_cache_shutdown(s);
-
-	if (__kmem_cache_shutdown(s) != 0)
-		return -EBUSY;
-
-	list_del(&s->list);
-
-	if (s->flags & SLAB_TYPESAFE_BY_RCU) {
-		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);
-	}
-
-	return 0;
-}
-
 void slab_kmem_cache_release(struct kmem_cache *s)
 {
 	__kmem_cache_release(s);
@@ -572,8 +546,8 @@ void slab_kmem_cache_release(struct kmem_cache *s)
 
 void kmem_cache_destroy(struct kmem_cache *s)
 {
-	int err = -EBUSY;
 	bool rcu_set;
+	int err;
 
 	if (unlikely(!s) || !kasan_check_byte(s))
 		return;
@@ -581,20 +555,45 @@ void kmem_cache_destroy(struct kmem_cache *s)
 	cpus_read_lock();
 	mutex_lock(&slab_mutex);
 
+	s->refcount--;
+	if (s->refcount) {
+		mutex_unlock(&slab_mutex);
+		cpus_read_unlock();
+		return;
+	}
+
 	rcu_set = s->flags & SLAB_TYPESAFE_BY_RCU;
 
-	s->refcount--;
-	if (s->refcount)
-		goto out_unlock;
+	/* free asan quarantined objects */
+	kasan_cache_shutdown(s);
 
-	err = shutdown_cache(s);
+	err = __kmem_cache_shutdown(s);
 	WARN(err, "%s %s: Slab cache still has objects when called from %pS",
 	     __func__, s->name, (void *)_RET_IP_);
-out_unlock:
+
+	if (!err)
+		list_del(&s->list);
+
 	mutex_unlock(&slab_mutex);
 	cpus_read_unlock();
-	if (!err && !rcu_set)
+
+	if (slab_state >= FULL) {
+		sysfs_slab_unlink(s);
+	}
+	debugfs_slab_release(s);
+
+	if (err)
+		return;
+
+	if (rcu_set) {
+		mutex_lock(&slab_mutex);
+		list_add_tail(&s->list, &slab_caches_to_rcu_destroy);
+		schedule_work(&slab_caches_to_rcu_destroy_work);
+		mutex_unlock(&slab_mutex);
+	} else {
+		kfence_shutdown_cache(s);
 		kmem_cache_release(s);
+	}
 }
 EXPORT_SYMBOL(kmem_cache_destroy);
 

-- 
2.45.2



  parent reply	other threads:[~2024-07-15 20:29 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-15 20:29 [PATCH RFC 0/6] mm, slub: handle pending kfree_rcu() in kmem_cache_destroy() Vlastimil Babka
2024-07-15 20:29 ` [PATCH RFC 1/6] mm, slab: make caches with refcount of 0 unmergeable Vlastimil Babka
2024-07-21  2:36   ` David Rientjes
2024-07-15 20:29 ` [PATCH RFC 2/6] mm, slab: always maintain per-node slab and object count Vlastimil Babka
2024-07-21  2:37   ` David Rientjes
2024-07-22 14:16   ` Xiongwei Song
2024-07-26 10:24     ` Vlastimil Babka
2024-07-15 20:29 ` Vlastimil Babka [this message]
2024-07-15 20:29 ` [PATCH RFC 4/6] mm, slab: simplify kmem_cache_release() Vlastimil Babka
2024-07-15 20:29 ` [PATCH RFC 5/6] mm, slab: asynchronously destroy caches with outstanding objects Vlastimil Babka
2024-07-15 20:29 ` [PATCH RFC 6/6] kunit, slub: add test_kfree_rcu() Vlastimil Babka
2024-07-21  2:39 ` [PATCH RFC 0/6] mm, slub: handle pending kfree_rcu() in kmem_cache_destroy() David Rientjes
2024-07-22  6:49   ` Vlastimil Babka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240715-b4-slab-kfree_rcu-destroy-v1-3-46b2984c2205@suse.cz \
    --to=vbabka@suse.cz \
    --cc=42.hyeyoo@gmail.com \
    --cc=Jason@zx2c4.com \
    --cc=Julia.Lawall@inria.fr \
    --cc=akpm@linux-foundation.org \
    --cc=boqun.feng@gmail.com \
    --cc=cl@linux.com \
    --cc=jiangshanlai@gmail.com \
    --cc=joel@joelfernandes.org \
    --cc=josh@joshtriplett.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=paulmck@kernel.org \
    --cc=qiang.zhang1211@gmail.com \
    --cc=rcu@vger.kernel.org \
    --cc=rientjes@google.com \
    --cc=roman.gushchin@linux.dev \
    --cc=rostedt@goodmis.org \
    --cc=urezki@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox