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

Breno Leitao
2026-04-24T12:43:18Z

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