diff mbox series

[1/2] Revert "mm/kmemleak: move the initialisation of object to __link_object"

Message ID 20231115082138.2649870-2-liushixin2@huawei.com (mailing list archive)
State New
Headers show
Series Fix invalid wait context of set_track_prepare() | expand

Commit Message

Liu Shixin Nov. 15, 2023, 8:21 a.m. UTC 
Move the initialisation of object back to__alloc_object() because
set_track_prepare() attempt to acquire zone->lock(spinlocks) while
__link_object is holding kmemleak_lock(raw_spinlocks). This is not
right for RT mode.

This reverts commit 245245c2fffd0050772a3f30ba50e2be92537a32.

Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
 mm/kmemleak.c | 36 +++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)

Comments

Geert Uytterhoeven Nov. 15, 2023, 8:19 a.m. UTC | #1 
On Wed, Nov 15, 2023 at 8:23 AM Liu Shixin <liushixin2@huawei.com> wrote:
> Move the initialisation of object back to__alloc_object() because
> set_track_prepare() attempt to acquire zone->lock(spinlocks) while
> __link_object is holding kmemleak_lock(raw_spinlocks). This is not
> right for RT mode.
>
> This reverts commit 245245c2fffd0050772a3f30ba50e2be92537a32.
>
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>

Reported-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert
Catalin Marinas Nov. 15, 2023, 2:17 p.m. UTC | #2 
On Wed, Nov 15, 2023 at 04:21:37PM +0800, Liu Shixin wrote:
> Move the initialisation of object back to__alloc_object() because
> set_track_prepare() attempt to acquire zone->lock(spinlocks) while
> __link_object is holding kmemleak_lock(raw_spinlocks). This is not
> right for RT mode.
> 
> This reverts commit 245245c2fffd0050772a3f30ba50e2be92537a32.
> 
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>

You can also add:

Fixes: 245245c2fffd ("mm/kmemleak: move the initialisation of object to __link_object")

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

I now realised that we update the object allocation stack trace via the
delete_object_part() when we shouldn't. I'd say __alloc_object() can
take a trace_handle as argument and if it's !0, set it directly whithout
calling set_track_prepare() (as a separate patch).
diff mbox series

Patch

diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 1eacca03bedd..22bab3738a9e 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -642,32 +642,16 @@  static struct kmemleak_object *__alloc_object(gfp_t gfp)
 	if (!object) {
 		pr_warn("Cannot allocate a kmemleak_object structure\n");
 		kmemleak_disable();
+		return NULL;
 	}
 
-	return object;
-}
-
-static int __link_object(struct kmemleak_object *object, unsigned long ptr,
-			 size_t size, int min_count, bool is_phys)
-{
-
-	struct kmemleak_object *parent;
-	struct rb_node **link, *rb_parent;
-	unsigned long untagged_ptr;
-	unsigned long untagged_objp;
-
 	INIT_LIST_HEAD(&object->object_list);
 	INIT_LIST_HEAD(&object->gray_list);
 	INIT_HLIST_HEAD(&object->area_list);
 	raw_spin_lock_init(&object->lock);
 	atomic_set(&object->use_count, 1);
-	object->flags = OBJECT_ALLOCATED | (is_phys ? OBJECT_PHYS : 0);
-	object->pointer = ptr;
-	object->size = kfence_ksize((void *)ptr) ?: size;
 	object->excess_ref = 0;
-	object->min_count = min_count;
 	object->count = 0;			/* white color initially */
-	object->jiffies = jiffies;
 	object->checksum = 0;
 	object->del_state = 0;
 
@@ -692,6 +676,24 @@  static int __link_object(struct kmemleak_object *object, unsigned long ptr,
 	/* kernel backtrace */
 	object->trace_handle = set_track_prepare();
 
+	return object;
+}
+
+static int __link_object(struct kmemleak_object *object, unsigned long ptr,
+			 size_t size, int min_count, bool is_phys)
+{
+
+	struct kmemleak_object *parent;
+	struct rb_node **link, *rb_parent;
+	unsigned long untagged_ptr;
+	unsigned long untagged_objp;
+
+	object->flags = OBJECT_ALLOCATED | (is_phys ? OBJECT_PHYS : 0);
+	object->pointer = ptr;
+	object->size = kfence_ksize((void *)ptr) ?: size;
+	object->min_count = min_count;
+	object->jiffies = jiffies;
+
 	untagged_ptr = (unsigned long)kasan_reset_tag((void *)ptr);
 	/*
 	 * Only update min_addr and max_addr with object