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

[Breno Leitao <leitao@debian.org>]

On Fri, Apr 24, 2026 at 01:05:20PM +0100, Catalin Marinas wrote:

> > @@ -153,6 +154,8 @@ struct kmemleak_object {
> >  	/* checksum for detecting modified objects */
> >  	u32 checksum;
> >  	depot_stack_handle_t trace_handle;
> > +	/* per-scan dedup count, valid only while in scan-local dedup xarray */
> > +	unsigned int dup_count;
> 
> I would add this around the pid_t pid member since both are 32-bit,
> better struct compaction. Here we'll get 32-bit padding.

Ack!

> > -	hex_dump_object(seq, object);
> > +	if (!no_hex_dump)
> > +		hex_dump_object(seq, object);
> 
> Nit: just use "hex_dump" and avoid double negation.

Ack!

> > +static void print_leak_locked(struct kmemleak_object *object, bool no_hex_dump)
> > +{
> > +	raw_spin_lock_irq(&object->lock);
> > +	if (object->flags & OBJECT_ALLOCATED)
> > +		__print_unreferenced(NULL, object, no_hex_dump);
> > +	raw_spin_unlock_irq(&object->lock);
> 
> I don't think OBJECT_ALLOCATED should prevent the printing here. If it's
> called from dedup_flush() and the first object that kept accumulating
> the dup_count is freed, you'd not print anything. I would only use
> OBJECT_ALLOCATED to decide whether to do the hex dump if requested.

That makes sense. I suppose we want something like:

  __print_unreferenced(NULL, object,
                       hex_dump && (object->flags & OBJECT_ALLOCATED));

Thanks for the review so far, I will respin the series,
--breno