Re: [PATCH 1/2] mm/kmemleak: dedupe verbose scan output by allocation backtrace

[Catalin Marinas <catalin.marinas@arm.com>]

On Tue, Apr 21, 2026 at 06:45:04AM -0700, Breno Leitao wrote:
> +/*
> + * Record a leaked object in the dedup table. The representative object's
> + * use_count is incremented so it can be safely dereferenced by dedup_flush()
> + * outside the RCU read section; dedup_flush() drops the reference. On
> + * allocation failure (or a concurrent insert) the object is printed
> + * immediately, preserving today's "always log every leak" guarantee.
> + * Caller must not hold object->lock and must hold rcu_read_lock().
> + */
> +static void dedup_record(struct xarray *dedup, struct kmemleak_object *object,
> +			 depot_stack_handle_t trace_handle)
> +{
> +	struct kmemleak_dedup_entry *entry;
> +
> +	entry = xa_load(dedup, trace_handle);
> +	if (entry) {
> +		/* This is a known beast, just increase the counter */
> +		entry->count++;
> +		return;
> +	}
> +
> +	/*
> +	 * A brand new report. Object will have object->use_count increased
> +	 * in here, and released put_object() at dedup_flush
> +	 */
> +	entry = kmalloc(sizeof(*entry), GFP_ATOMIC);

Do we need to allocate a structure here? We could instead add a
dup_count member in the kmemleak_object and just link the object itself
into the xarray. Well, maybe the leak being a rare event is not that
bad.

> +	if (entry && get_object(object)) {
> +		if (xa_insert(dedup, trace_handle, entry, GFP_ATOMIC) == 0) {

I wonder if we need xa_insert() at all. Since it's indexed by
trace_handle, we could follow similar mechanism like stack_depot with a
large hash array, maybe gated by CONFIG_DEBUG_KMEMLEAK_VERBOSE.

> +			entry->object = object;
> +			entry->count = 1;
> +			return;
> +		}
> +		put_object(object);
> +	}
> +	kfree(entry);
> +
> +	/*
> +	 * Fallback for kmalloc/get_object(): Just print it straight away
> +	 */
> +	raw_spin_lock_irq(&object->lock);
> +	print_unreferenced(NULL, object);
> +	raw_spin_unlock_irq(&object->lock);
> +}
> +
> +/*
> + * Drain the dedup table: print one full record per unique backtrace,
> + * followed by a summary line whenever more than one object shared it.
> + * Releases the reference dedup_record() took on each representative object.
> + */
> +static void dedup_flush(struct xarray *dedup)
> +{
> +	struct kmemleak_dedup_entry *entry;
> +	unsigned long idx;
> +
> +	xa_for_each(dedup, idx, entry) {
> +		raw_spin_lock_irq(&entry->object->lock);
> +		print_unreferenced(NULL, entry->object);
> +		raw_spin_unlock_irq(&entry->object->lock);

Sashiko has a good point here - while the kmemleak metadata is still
around due to an earlier get_object(), the object itself may have been
freed and the hex dump in print_unreferenced() could fault (e.g.
vunmap'ed object). Same with the print_unreferenced() above. It's
probably not worth printing the first bytes of the content anyway when
we do coalescing, the content would differ anyway. Also it's possible
that the size differs even if the stack trace is the same but I guess we
can ignore this.

https://sashiko.dev/#/patchset/20260421-kmemleak_dedup-v1-0-65e31c6cdf0c@debian.org

-- 
Catalin