[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZS_81Oy5S-VeLrT3@arm.com>
Date: Wed, 18 Oct 2023 16:42:12 +0100
From: Catalin Marinas <catalin.marinas@....com>
To: Liu Shixin <liushixin2@...wei.com>
Cc: Patrick Wang <patrick.wang.shcn@...il.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Kefeng Wang <wangkefeng.wang@...wei.com>, linux-mm@...ck.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 4/7] mm: kmemleak: split __create_object into two
functions
Thanks for this, it looks better.
On Wed, Oct 18, 2023 at 06:29:49PM +0800, Liu Shixin wrote:
> -/*
> - * Create the metadata (struct kmemleak_object) corresponding to an allocated
> - * memory block and add it to the object_list and object_tree_root (or
> - * object_phys_tree_root).
> - */
> -static void __create_object(unsigned long ptr, size_t size,
> - int min_count, gfp_t gfp, bool is_phys)
> +static struct kmemleak_object * __alloc_object(gfp_t gfp)
> {
> - unsigned long flags;
> - struct kmemleak_object *object, *parent;
> - struct rb_node **link, *rb_parent;
> - unsigned long untagged_ptr;
> - unsigned long untagged_objp;
> + struct kmemleak_object *object;
>
> object = mem_pool_alloc(gfp);
> if (!object) {
> pr_warn("Cannot allocate a kmemleak_object structure\n");
> kmemleak_disable();
> - return;
> + return NULL;
> }
>
> INIT_LIST_HEAD(&object->object_list);
> @@ -649,13 +639,8 @@ static void __create_object(unsigned long ptr, size_t size,
> 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;
I'd keep all the initialisation in one place even if it means passing
more arguments to __alloc_object(). It feels a bit weird and error prone
to split the initialisation in two places. Otherwise I'm fine with the
split.
--
Catalin
Powered by blists - more mailing lists