From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (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 4837821ABAA for ; Sun, 21 Jun 2026 16:14:57 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782058498; cv=none; b=ZNkR7j/5BAQ6sfAfzuQA4o3fJsTK4/+szgxALZ9svm/4bXeJ6QvHuEF9CE2rklilz39vPq1ZuA6TOT5c/m4bLCDpbVEjDPDmyC75c7hsY5bXvNK4BarP5CzGWI16wejn4QVvCTe/RzJ23qVR6UsqG9IRaBO/HPMw+pp8herTNwI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782058498; c=relaxed/simple; bh=2jHGOG8DfaTcddaFLHpDt0fgpUrZc62g8EX5cwzbVgs=; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date: Message-Id; b=Le503GOCtJ2MFjFGEgBgmU+ZG+eAHDHaZqm765go6QU0I9T1ok74VLovzPHofy2LfJ+fvkk8eILEH4jsscfFejb9rBqflsUEbFTuYXsV8RHd1EHL9rSqr7NTsOVewOukMAqF6NYeYasTlfzsRsfvGo01zznWHS0VoFqnpwncM1k= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=k6LHS6T1; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="k6LHS6T1" Received: by smtp.kernel.org (Postfix) with ESMTPSA id F23981F000E9; Sun, 21 Jun 2026 16:14:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1782058497; bh=Fpbe5nG8dfrNbR3sw756/1qGIaJdiKKMdYm+I8cv+XY=; h=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date; b=k6LHS6T1j+EF33Ac7DJAbv5sF3WeBJo8sw9YHMKbNDkRxDYahh7fLs5YLEDAZwjVr Aoocd7+k8q1Zt5FKBuzllHTiRiSAl+nHij6o+Bv4AyM82ZTnbpR3KkO4aLi+vqlZzm XEy3KJQHfnPcywQlKZEbXKZfjQgwcIZXm9j8sA85sX6tvWuzBaqqHJIyg6OW7v+A5w Iuks/ExW0am42KYfmCq2RN8tczFR+Oh5CKMLm/hM1RgLKmsEil5zNDuh9aBmvy4sfd IYjpihyIcx8KwflI0meWQuwSnIzki2ejFbDJy+VJJS8BLFejNa6KFNdxrZ7bMSfKIl DTYb9l6TV1QwA== From: sashiko-bot@kernel.org Subject: Re: [RFC PATCH v1.2 13/17] mm/damon/core: remove attrs param from damon_update_region_access_rate() Reply-To: sashiko-reviews@lists.linux.dev To: "SeongJae Park" Cc: damon@lists.linux.dev In-Reply-To: <20260621155715.87932-14-sj@kernel.org> References: <20260621155715.87932-1-sj@kernel.org> <20260621155715.87932-14-sj@kernel.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Sun, 21 Jun 2026 16:14:56 +0000 Message-Id: <20260621161456.F23981F000E9@smtp.kernel.org> Precedence: bulk X-Mailing-List: damon@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: Thank you for your contribution! Sashiko AI review found 1 potential issue(= s) to consider: Pre-existing issues: - [High] Unprotected static local variables in access check functions cause= cross-thread data races and cross-context access measurement pollution. -- commit cc8a115fada6e1595d5b8a6c2e994e38ac982a9c Author: SeongJae Park mm/damon/core: remove attrs param from damon_update_region_access_rate() This commit removes the unused attrs parameter from damon_update_region_access_rate() and updates callers in the core, physical address, and virtual address monitoring code to match the new signature. [ ... ] > diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c > index d0598f5f26882..db48ad274b078 100644 > --- a/mm/damon/paddr.c > +++ b/mm/damon/paddr.c > @@ -93,12 +93,12 @@ static void __damon_pa_check_access(struct damon_regi= on *r, > /* If the region is in the last checked page, reuse the result */ > if (ALIGN_DOWN(last_addr, last_folio_sz) =3D=3D > ALIGN_DOWN(sampling_addr, last_folio_sz)) { > - damon_update_region_access_rate(r, last_accessed, attrs); > + damon_update_region_access_rate(r, last_accessed); [Severity: High] This is a pre-existing issue, but does relying on last_accessed here cause data races when multiple kdamond threads run concurrently? In __damon_pa_check_access(), last_addr, last_folio_sz, and last_accessed are defined as static variables. Because they are shared globally across all concurrent damon contexts without synchronization, multiple kdamond threads will read and write them simultaneously. Could this allow one thread to incorrectly reuse a last_accessed result generated by a completely different thread monitoring a different physical region, corrupting the measurement accuracy? > return; > } [ ... ] > diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c > index d271476035641..2e12cc628fea0 100644 > --- a/mm/damon/vaddr.c > +++ b/mm/damon/vaddr.c > @@ -503,19 +503,19 @@ static void __damon_va_check_access(struct mm_struc= t *mm, > static bool last_accessed; [Severity: High] This isn't a bug introduced by this patch, but this static variable shares = the same cross-thread data race issue as the physical address check. If multiple contexts are monitoring different processes, they will write to these variables simultaneously. Does this data race allow one kdamond thread to accidentally reuse the last_accessed result from another thread, breaking context isolation? > =20 > if (!mm) { > - damon_update_region_access_rate(r, false, attrs); > + damon_update_region_access_rate(r, false); > return; > } --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260621155715.8793= 2-1-sj@kernel.org?part=3D13