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
