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 EDACA5CDF1 for ; Fri, 26 Jun 2026 01:02:02 +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=1782435724; cv=none; b=uPXtBWee0k9GDkusIg3fw3YHpxzb6qmkGWETm6DQMV6BceSd2bW78/OO9ma7ZWr/REEBCsIAmE+cHkUhVHyk9/A1hLapO57t0URac9aC6MHFLu1F9YWmW+yxJ7olDZ3QWu4LXKQtB8b/+gXn8kU7fzUVI38hDz03jU6h8PFVDJo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782435724; c=relaxed/simple; bh=cNFjZMMGrqBJ7XIpcj3/dYDDCftmyKmp7P10wuud1No=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=dGEVNcS9ctJjrx5VfSdKM31gjqPOG0pWoYtxk+BSHQu2D96ft5UOpa//wKYQcRqVOHDDV9UTV4dLIxUS51pLhpLRhfeRcloRYNzJUd+nAt3+Ft66N9s4KbAACLFmaQAyR81jTq84RrSCEQAtvkPkUihThfJxMeimfKkjo6Ql6Po= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=WKPhqbi9; 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="WKPhqbi9" Received: by smtp.kernel.org (Postfix) with ESMTPSA id BC2651F000E9; Fri, 26 Jun 2026 01:02:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1782435722; bh=xLXBq7GQfyyZQ6fwdexfQMyw1Cu9bLT+TTRx5LImcf8=; h=From:To:Cc:Subject:Date:In-Reply-To:References; b=WKPhqbi93AZufo4RCfMio9F8N4WmLqyfJltZBfAZyVELqu8h6wxpSqKhH5x9NC/Wc DqD49HCt83btb6qi7TvFRCc+XAiF5hwNlXnVFepWRK0h5+O5fsfwlF3O2ehaD2kAG6 VSYGp19EDPRD3iv1e7DV5z4LLywXyOv0OS0G9X6Ac5gh4eKO3NPWJUbSYqqfQsFxxH +33b0TaO9cp0Fwgd5bqvVKksyPq1oFWiFIwVnANokE14fYcAVvPr6jSQCFj4ZQezu7 543HMu05PQS+sMHzuxrPjxeTw768p16jWN4ZNcDyomterKQz0cewHspV73+Bszf8ie yPkBSqbwK3Xcw== From: SeongJae Park To: SeongJae Park Cc: sashiko-bot@kernel.org, damon@lists.linux.dev Subject: Re: [RFC PATCH v1.2 13/17] mm/damon/core: remove attrs param from damon_update_region_access_rate() Date: Thu, 25 Jun 2026 18:01:52 -0700 Message-ID: <20260626010152.90914-1-sj@kernel.org> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260621204050.10993-1-sj@kernel.org> References: Precedence: bulk X-Mailing-List: damon@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit On Sun, 21 Jun 2026 13:40:49 -0700 SeongJae Park wrote: > On Sun, 21 Jun 2026 16:14:56 +0000 sashiko-bot@kernel.org wrote: > > > 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_region *r, > > > /* If the region is in the last checked page, reuse the result */ > > > if (ALIGN_DOWN(last_addr, last_folio_sz) == > > > 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? > > Nice finding. Not a blocker of this patch, though. I will separatelly work on > this. In the second thought, the real problem of the race is quite doubtful. Only a few access check will be corrupted, when there are multiple kdamonds that monitoring overlapping physical address ranges. It is user's responsibility to avoid this kind of inteference, anyway. In vaddr case, the race can happen if the user sets the kdamonds to monitor different virtual address spaces, though. But, still, corrupting a few accesses is not a big deal in DAMON world. Meanwhile, this code was introduced as an overhead optimization. The gain from the optimization is also doubtful. DAMON will anyway shape the regions to not cross folio in normal cases. Only one thing that very clear to me is, the code looks dirty. I will simply dropp this optimization with no urgency. It will be mainly for code readability improvement rather than a bug fix. Hence no Fixes: will be added, and will not Cc stable@. It may alos be posted relatively slowly, as one of usual misc cleanup series, in the later part of 7.3 development cycle, or the earlier part of 7.4 development cycle. Let me know if anyone think differently. Thanks, SJ [...]