From patchwork Wed Feb 26 06:36:41 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: SeongJae Park X-Patchwork-Id: 13991592 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id E697CC021BC for ; Wed, 26 Feb 2025 06:37:02 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id AC8E428000B; Wed, 26 Feb 2025 01:37:01 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id A2734280007; Wed, 26 Feb 2025 01:37:01 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 87C3228000B; Wed, 26 Feb 2025 01:37:01 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 64E1F280007 for ; Wed, 26 Feb 2025 01:37:01 -0500 (EST) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 16492C0C8B for ; Wed, 26 Feb 2025 06:37:01 +0000 (UTC) X-FDA: 83161138242.12.C08D431 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf26.hostedemail.com (Postfix) with ESMTP id 6A259140005 for ; Wed, 26 Feb 2025 06:36:59 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=c4OrK71w; spf=pass (imf26.hostedemail.com: domain of sj@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=sj@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1740551819; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=B7aBgOL4eIwD0I00Rj2bBgoF2ZdNknG/Et03R5aS6jc=; b=XiPsUtUp/Zifj02x2fEepuNUgTg2mHTLmHvvH7VKnT8WKs87tefdCXMR2x9hIcn2xtyzTX pBjekiOSRSIkzKUlMpKXdkG3umXZIRG5ITKslA7oYcrwUe2ZGytGBMgIORAycNCaae2jbg e1ucgK3nKC5D5LBNUGsZzYxrDuelQtE= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=c4OrK71w; spf=pass (imf26.hostedemail.com: domain of sj@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=sj@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1740551819; a=rsa-sha256; cv=none; b=cZoUZdWKbPtTE0FUuF7V8Yv5RBzR8fQs272863eioQpM50VrJ+GSr/l97DNBUKEpzwBG85 wlh6uF5CmMfOyRnGnHy7ZN0Bzz+hcP5nRNji46yzapdziGBX15jZ8UdWfYHnRvMRNjmteg wA3yvUvRrvejOXV0aS2HxOwDDitgL+U= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id BF2D15C5A46; Wed, 26 Feb 2025 06:36:19 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 23684C4CEEA; Wed, 26 Feb 2025 06:36:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1740551818; bh=43qAgqvlrGuT5QCdJ6rPL+omeoO3HK7C7mVfbHXX3rQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=c4OrK71wA49aXqNHD0dgxHKUbfmC0RMtGyCLCaB6W3Eyv4rKbWxPGCoR1QCw0SNg9 hjHvjPnTQAqPS3kKuYHaXiKVTPYlH1ij1VFFgXXiSPsfrzOJ2suCfPduH0d6nzK20g hBBC+6wchQZVwWHdiszEe/DQwIcEXs/YKGslXcd0k2jL3BXRBtBC8LPDS778Vr6lvY kazGYDDVN7rsJKJ6BEtfvgINPlqmFKOoMkaq3G+lbsyOlwAlRx1jE4kS66PMnFypWU Xm9TfZgjYCUQ4B+nYySeAzTOQb3myWea8idoUNYg+f77E66mcHOWHMf7O+H2UXCwO+ EVNsmFbEdpE4A== From: SeongJae Park To: Cc: SeongJae Park , Andrew Morton , damon@lists.linux.dev, kernel-team@meta.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: [RFC PATCH 03/13] mm/damon/core: make damon_set_attrs() be safe to be called from damon_call() Date: Tue, 25 Feb 2025 22:36:41 -0800 Message-Id: <20250226063651.513178-4-sj@kernel.org> X-Mailer: git-send-email 2.39.5 In-Reply-To: <20250226063651.513178-1-sj@kernel.org> References: <20250226063651.513178-1-sj@kernel.org> MIME-Version: 1.0 X-Rspam-User: X-Rspamd-Server: rspam11 X-Rspamd-Queue-Id: 6A259140005 X-Stat-Signature: 5tufaxaxzpyq61uphnq1og8ge5dkxysm X-HE-Tag: 1740551819-328936 X-HE-Meta: U2FsdGVkX19hlbdWOTqLqkNKDRNK0PnGgjwE9mf6wUMABCBTg80rzrN/0U8ABNgb+dRXVh4vpkTV7kwF8YxxnWJzTeoHlya0obvBTd6tMfRvPK0FSJOHKFl3xCp/+Nv/OXLA6y3tTI8Jxl5TRMlouNwcx0/N3/JiGl8YSl3BH8bY2zLUWtn4MYmdfEhiQPYmf0f9uC5x+Aiw6QLotL1yMQmHAb63ChO8IL0zMSlxvekl+ffbNdN8C5jsABMfqUPR1FTQuQiLmlNyEHeJgZt91uaTAXBeX9WruLqVtRa8JFjmYZAbVHgJtZ9B8qI1Lx0e5smW3FtqXQbrAychRNBmAbGY/F48O5th0Bx/HXyLvSKkUPFfbq8xamKkOlXvaTYgGY+g/AYH4QWxixlllSAWcXRj5kh/NWuAJM9h7ulsbNB2qUrE7Gda4A7NClokDx7s4jEHM4DzCmqToeo3hsQNeeSpdhnmtTKuJuyt0pfoH4trOOdC9Tjgjbagy7k5+887HXCfbKrpHk6cDwaqJO6bY+2G1qoNn8LhLutyTJ7NmNHBX3Bk/NnpIdpWr94J4aKCi2+O9mQbUtrFi68UKaf3WkdrtqaohrHf//FzI2qC5fgl/FAYXvzco1gk0uiQyG4Ndw+3RHRZC9yREgKlQoLOI96JwXZoLqLSWIa/QbxvP+TUcRMffo/JPK7ktH1chyW7XXyLnMWYiMeF0qaauf3bC/FdAumYUidbT3buxYzMU/vygXuI2RPJu5RU1u3jfAi6vRJ9PncUN/7jm+N0f5ZCaCxt0s0NnK6oTNxA8Kz7btk5mBx+dSEWJlVGBK3+pM/nZpai89TGI8TeJQF6qkfq6oSa872rYi2iDfYpqFoSyLvMTZQbXrWOqh7+QIMsh/ub1rCQ2LUj9NtqkMDeYKJt5vVvM3kk0+UC5WzOXc8HjjL9/JTMC1VY2lBfWq9EqPzerYFc3fTYCyq/WfkEo1O LGWzC4PP Z1oMccFKbi4lcjJfNFjPyA+TtvBpt+F8UZIFqcHSZtS4Z4e9I2dpeO2V6Cq9xKo9AVjllR1S+CgzmgyN1BwdFiWcoe4i7C2Zj8tOJdemkZbvdF3GQix43r0DUwTobDWsvlRU6n+R6WwMamPae3EeLUzTqlAWU5a9seV7cVUjv7wvlksDZ93tgDrozDrfXejI2Mz29Bm3OHNxteu+1BhnYNCuHqys0UbPSn3Vxvm10KaUuM4JBtHkyZqZZjjXLknTUnhrOpPOvutyxnddSAP6DhGfMgQ== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: Currently all DAMON kernel API callers do online DAMON parameters commit from damon_callback->after_aggregation because only those are safe place to call the DAMON monitoring attributes update function, namely damon_set_attrs(). Because damon_callback hooks provides no synchronization, the callers works in asynchronous ways or implement their own inefficient and complicated synchronization mechanisms. It also means online DAMON parameters commit can take up to one aggregation interval. On large systems having long aggregation intervals, that can be too slow. The synchronization can be done in more efficient and simple way while removing the latency constraint if it can be done using damon_call(). The fact that damon_call() can be executed in the middle of the aggregation makes damon_set_attrs() unsafe to be called from it, though. Two real problems can occur in the case. First, converting the not yet completely aggregated nr_accesses for new user-set intervals can arguably degrade the accuracy or at least make the logic complicated. Second, kdamond_reset_aggregated() will not be called after the monitoring results update, so next aggregation starts from unclean state. This can result in inconsistent and unexpected nr_accesses. Make it safe as follows. Catch the middle-of-the-aggregation case from damon_set_attrs() and pass the information to nr_accesses conversion logic. The logic works as before if this is not the case (called after the current aggregation is completed). If not, it drops the nr_accesses information that so far aggregated, and make the status same to the beginning of this aggregation, but as if the last aggregation was ran with the updated sampling/aggregation intervals. Signed-off-by: SeongJae Park --- mm/damon/core.c | 38 ++++++++++++++++++++++++++----------- mm/damon/tests/core-kunit.h | 6 +++--- 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/mm/damon/core.c b/mm/damon/core.c index 0578e89dff13..5b807caaec95 100644 --- a/mm/damon/core.c +++ b/mm/damon/core.c @@ -602,11 +602,25 @@ static unsigned int damon_nr_accesses_for_new_attrs(unsigned int nr_accesses, } static void damon_update_monitoring_result(struct damon_region *r, - struct damon_attrs *old_attrs, struct damon_attrs *new_attrs) + struct damon_attrs *old_attrs, struct damon_attrs *new_attrs, + bool aggregating) { - r->nr_accesses = damon_nr_accesses_for_new_attrs(r->nr_accesses, - old_attrs, new_attrs); - r->nr_accesses_bp = r->nr_accesses * 10000; + if (!aggregating) { + r->nr_accesses = damon_nr_accesses_for_new_attrs( + r->nr_accesses, old_attrs, new_attrs); + r->nr_accesses_bp = r->nr_accesses * 10000; + } else { + /* + * if this is called in the middle of the aggregation, reset + * the aggregations we made so far for this aggregation + * interval. In other words, make the status like + * kdamond_reset_aggregated() is called. + */ + r->last_nr_accesses = damon_nr_accesses_for_new_attrs( + r->last_nr_accesses, old_attrs, new_attrs); + r->nr_accesses_bp = r->last_nr_accesses * 10000; + r->nr_accesses = 0; + } r->age = damon_age_for_new_attrs(r->age, old_attrs, new_attrs); } @@ -619,7 +633,7 @@ static void damon_update_monitoring_result(struct damon_region *r, * ->nr_accesses and ->age of given damon_ctx's regions for new damon_attrs. */ static void damon_update_monitoring_results(struct damon_ctx *ctx, - struct damon_attrs *new_attrs) + struct damon_attrs *new_attrs, bool aggregating) { struct damon_attrs *old_attrs = &ctx->attrs; struct damon_target *t; @@ -634,7 +648,7 @@ static void damon_update_monitoring_results(struct damon_ctx *ctx, damon_for_each_target(t, ctx) damon_for_each_region(r, t) damon_update_monitoring_result( - r, old_attrs, new_attrs); + r, old_attrs, new_attrs, aggregating); } /* @@ -661,10 +675,10 @@ static bool damon_valid_intervals_goal(struct damon_attrs *attrs) * @ctx: monitoring context * @attrs: monitoring attributes * - * This function should be called while the kdamond is not running, or an - * access check results aggregation is not ongoing (e.g., from - * &struct damon_callback->after_aggregation or - * &struct damon_callback->after_wmarks_check callbacks). + * This function should be called while the kdamond is not running, an access + * check results aggregation is not ongoing (e.g., from &struct + * damon_callback->after_aggregation or &struct + * damon_callback->after_wmarks_check callbacks), or from damon_call(). * * Every time interval is in micro-seconds. * @@ -675,6 +689,8 @@ int damon_set_attrs(struct damon_ctx *ctx, struct damon_attrs *attrs) unsigned long sample_interval = attrs->sample_interval ? attrs->sample_interval : 1; struct damos *s; + bool aggregating = ctx->passed_sample_intervals < + ctx->next_aggregation_sis; if (!damon_valid_intervals_goal(attrs)) return -EINVAL; @@ -695,7 +711,7 @@ int damon_set_attrs(struct damon_ctx *ctx, struct damon_attrs *attrs) ctx->next_ops_update_sis = ctx->passed_sample_intervals + attrs->ops_update_interval / sample_interval; - damon_update_monitoring_results(ctx, attrs); + damon_update_monitoring_results(ctx, attrs, aggregating); ctx->attrs = *attrs; damon_for_each_scheme(s, ctx) diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h index 532c6a6f21f9..be0fea9ee5fc 100644 --- a/mm/damon/tests/core-kunit.h +++ b/mm/damon/tests/core-kunit.h @@ -348,19 +348,19 @@ static void damon_test_update_monitoring_result(struct kunit *test) new_attrs = (struct damon_attrs){ .sample_interval = 100, .aggr_interval = 10000,}; - damon_update_monitoring_result(r, &old_attrs, &new_attrs); + damon_update_monitoring_result(r, &old_attrs, &new_attrs, false); KUNIT_EXPECT_EQ(test, r->nr_accesses, 15); KUNIT_EXPECT_EQ(test, r->age, 2); new_attrs = (struct damon_attrs){ .sample_interval = 1, .aggr_interval = 1000}; - damon_update_monitoring_result(r, &old_attrs, &new_attrs); + damon_update_monitoring_result(r, &old_attrs, &new_attrs, false); KUNIT_EXPECT_EQ(test, r->nr_accesses, 150); KUNIT_EXPECT_EQ(test, r->age, 2); new_attrs = (struct damon_attrs){ .sample_interval = 1, .aggr_interval = 100}; - damon_update_monitoring_result(r, &old_attrs, &new_attrs); + damon_update_monitoring_result(r, &old_attrs, &new_attrs, false); KUNIT_EXPECT_EQ(test, r->nr_accesses, 150); KUNIT_EXPECT_EQ(test, r->age, 20);