Re: [PATCH RFC] mm/kmemleak: avoid soft lockup when scanning task stacks

[Lance Yang <lance.yang@linux.dev>]

On 2026/6/12 17:09, Breno Leitao wrote:
> Hello Lance,
> 
> First of all, thanks for ther review, really awesome!

Cool!

> On Fri, Jun 12, 2026 at 11:16:05AM +0800, Lance Yang wrote:
>> On Thu, Jun 11, 2026 at 05:45:00AM -0700, Breno Leitao wrote:
>>> kmemleak_scan() walks every thread and scans its kernel stack under a
>>> single rcu_read_lock() with no reschedule point. On a host with very
>>> many threads -- amplified by KASAN/lockdep in debug builds -- this loop
>>> can hog a CPU long enough to trip the soft lockup watchdog:
>>>
>>>   watchdog: BUG: soft lockup - CPU#35 stuck for 22s! [kmemleak:537]
>>>    scan_block
>>>    kmemleak_scan
>>>    kmemleak_scan_thread
>>>    kthread
>>
>> Neat, good catch!
>>
>>> A cond_resched() cannot be added directly: the loop runs inside an RCU
>>> read-side critical section.
>>>
>>> Split the scan in two parts:
>>>
>>> 1) get the list of tasks (with RCU read lock) in an array
>>> 2) run scan_block() for the tasks (with cond_reschd()).
>>>
>>> Is it a sane approach?
>>
>> Why not use the kernel/hung_task.c pattern here? Seems simpler, with no
>> extra task-array allocation ;)
> 
> I've looked at it, but I am not sure we want to break the loop mid-air,
> that seems to increase the false positives, given we did a half-baked
> scan, right?
> 
>> Could break RCU only when resched is needed. Pin the current cursors,
>> drop RCU, cond_resched(), take RCU again, and continue only if the
>> cursors are still alive ;)
>>
>> If either cursor died while RCU was droped, stopping this scan round
>> should be fine, IMHO.
> 
> I am not sure, this is not the same as the existing kmemleak_cond_resched()
> raciness in the object_list loops. Those iterate the marked set, where a miss
> only means "this object isn't reported until the next scan" -- under-reporting,
> self-healing, and the in-tree comment says exactly that.
> 
> Dropping a *root* mid-scan is the opposite: it makes *other* objects get
> falsely reported. So the "it's already racy, bailing is fine" reasoning doesn't
> carry over from the object loop to the stack loop.
> 
> If we go this route, the aborted round has to suppress reporting, reusing
> kmemleak's existing "scan was interrupted -> don't report" path:
> 
>        if (need_resched() && !kmemleak_stack_scan_break(g, p)) {
>                aborted = true;
>                goto unlock;
>        }

I'd expect the normal case to just drop RCU, cond_resched(), take RCU
again, see both cursors still alive, and keep walking :)

>        ...
>        if (scan_should_stop() || aborted)
>                return;

And yeah, you're right. If we do lost a cursor, bailing out and
skipping reporting fot that incomplete root scan should be the
right thing, I guess :D

> Then an abort means "this round reports nothing; the next full scan
> reports the real leaks" instead of a false-positive flood.
> 
> On boxes with very many threads, where the stack walk is long and
> need_resched() fires constantly, so the break helper runs a lot -- which makes
> aborts (and thus fully-suppressed, non-reporting rounds) plausibly more than
> "rare".
> 
> Since each round restarts from the head, the tail of the thread list is the
> most likely to be perpetually skipped, on exactly the workload this is meant to
> fix.
> 
> The snapshot avoids that by scanning a complete, similar to what we have today.
> 
> Anyway, I would love to get rid of the array, but, I am not convinced that
> dropping the scan mid-air will not cause false positives.
> 
> Thanks for the review,

Cheers!