From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-171.mta0.migadu.com (out-171.mta0.migadu.com [91.218.175.171]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 040E2303A04 for ; Fri, 12 Jun 2026 09:57:21 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.171 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781258244; cv=none; b=lFnAhdsxjiAgv0bGuySuE8ICT8NNmjbhN1LY0ZAeht/7EhK351AjPjz8PI9WPiOIE3uo3tUs3ieAQICVcQUhMIO3c8ct0n0E7qdL40HuSOb77WR1jMhNrpxDTuOmtszqbnfkNZ6I21KpzFwFgMw+WhKvGRg54MOhut2M7a3a7GI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781258244; c=relaxed/simple; bh=eOU2Gr9pxdNcG07cURZp/1nHxcmToPBpoBOodRxd+FY=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=qo5lvQ4ca7NuqvGhxJ2u7mk8iG7tQf469QuK+ghErSqsj54dU7TmNIecLTHjHg/xFrjJS3wRiT3+zIyt3sz9zGuzJqEcnLS+UjrX533eYkcKq+boJtzMNnoJ5P5/IBEpvntmGiigSEarSW1tRr7L+0MEHEtCwCoJc+uM0hzbDY8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=ZOah2sde; arc=none smtp.client-ip=91.218.175.171 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="ZOah2sde" Message-ID: <6bc446b7-8e25-4add-9d72-a3c9e9191533@linux.dev> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1781258239; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=40ldlKf5NOiyVD+LSgw0DbXbrN4Mt4O2B8RW5UMcBu4=; b=ZOah2sdeCuUGurCIybhtURWxH+ixhU/M4kIZymHrTnh2TYnA+vn/hWbbamX9WGdrRoOPmJ o1axy3sETPr0P5hOR2ShCEtWCvLO+HIgHTT8WummMAqttk0zCGfJazkYvXTou9ClxrSeZb tVoYqx4oHxW9/Y2yvGuQTA4YT+eKUDw= Date: Fri, 12 Jun 2026 17:57:12 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Subject: Re: [PATCH RFC] mm/kmemleak: avoid soft lockup when scanning task stacks Content-Language: en-US To: Breno Leitao Cc: catalin.marinas@arm.com, akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, kernel-team@meta.com, sj@kernel.org References: <20260611-kmemleak-stack-resched-v1-1-d6248ade5f4a@debian.org> <20260612031605.58235-1-lance.yang@linux.dev> X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Lance Yang In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Migadu-Flow: FLOW_OUT 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!