Re: + radix-tree-fix-kmemleak-false-positives-on-tree-head-reassignment.patch added to mm-new branch

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

Hi Breno,

Thanks looking looking at this.

On Tue, Jul 07, 2026 at 04:26:43AM -0700, Breno Leitao wrote:
> On Tue, Jul 07, 2026 at 12:19:50AM +0100, Catalin Marinas wrote:
> > +static bool flag_suspects(void)
> > +{
> > +	struct kmemleak_object *object;
> > +	int suspects = 0;
> > +
> > +	rcu_read_lock();
> > +	list_for_each_entry_rcu(object, &object_list, object_list) {
> 
> __kmemleak_scan() above alread iterated over these objects. Is it
> possible to piggy back on __kmemleak_scan() list_for_each_entry_rcu
> for the objects to flag SUSPECTS?

That list has another scan_gray_list() afterwords. We could fold it into
the same list - flag all white objects here as suspects and clear this
flag in update_refs() if the object becomes gray. We'll probably need a
counter of suspected leaks set during this loop and decrement it during
update_refs() when an object becomes gray. Only go for a second scan if
this number is positive.

Yet another option would be for __kmemleak_scan() to return true if it
found any new candidates in this loop (new white objects that could not
be added to the gray list). It does not remove the second loop but it
makes it less likely for a system that doesn't have new leaks.

BTW, I think the flag_suspects() loop was missing an early continue if
!colour_white(), saves from taking the object look. But it still does a
list walk and I can see both loops looking pretty similar that one
wonders why they shouldn't be merged.

> > +		raw_spin_lock_irq(&object->lock);
> > +		if (unreferenced_object(object) &&
> > +		    !(object->flags & OBJECT_REPORTED)) {
> > +			object->flags |= OBJECT_SUSPECT;
> > +			suspects++;
> > +		} else {
> > +			object->flags &= ~OBJECT_SUSPECT;
> > +		}
> > +		raw_spin_unlock_irq(&object->lock);
> > +		if (need_resched())
> > +			kmemleak_cond_resched(object);
> > +	}
> > +	rcu_read_unlock();
> > +
> > +	return suspects != 0;
> > +}
> > +
> > +/*
> > + * Scan the memory and report the unreferenced objects as leaks. Must be
> > + * called with the scan_mutex held.
> > + */
> > +static void kmemleak_scan(void)
> > +{
> > +	struct kmemleak_object *object;
> > +	struct xarray dedup;
> > +	int new_leaks = 0;
> 
> It seems new_leaks and dedup are not used at all.

They are in kmemleak_scan() after the second __kmemleak_scan() call.

> reviewing this code, I found that unreferenced_object(object)) is always
> called/checked with (object->flags & OBJECT_REPORTED). Would it be ok to
> move (object->flags & OBJECT_REPORTED) to inside unreferenced_object()?

They are meant for different things. The flag is for reporting and
debugfs wants REPORTED, kmemleak_scan() checks for unreferenced and
!REPORTED,

-- 
Catalin