From mboxrd@z Thu Jan 1 00:00:00 1970 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 E8CE5C71153 for ; Wed, 30 Aug 2023 01:51:17 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 66FD28E0041; Tue, 29 Aug 2023 21:51:17 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 61F5D8E000B; Tue, 29 Aug 2023 21:51:17 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 50E268E0041; Tue, 29 Aug 2023 21:51:17 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 414568E000B for ; Tue, 29 Aug 2023 21:51:17 -0400 (EDT) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id F3BAA1403E1 for ; Wed, 30 Aug 2023 01:51:16 +0000 (UTC) X-FDA: 81179093352.21.B652E39 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf29.hostedemail.com (Postfix) with ESMTP id 5F126120009 for ; Wed, 30 Aug 2023 01:51:15 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=J9ujTFhF; spf=pass (imf29.hostedemail.com: domain of sj@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=sj@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1693360275; 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=28rHNFuIwAnv/v3I68a2QQ6Afi9wHowc5T5mUI4NIhA=; b=UMjWmSys7w/E2xydn6oRERhpdWCoo1N7BiBIrxKUe5AVbX569bssVNdEMWCo940UlUTsVn k3TaAEwxigTSQim0Hxk8ZbVoJlCGpyeHLZZpwxyfw08gSjJ/ghc4KwnQwa2Cl0UpiSSdoD vzWHLLlJ5pdV5PF1mILcFm0unTPPq8w= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1693360275; a=rsa-sha256; cv=none; b=v0wvYaoXOdWs+vR+1w8Z2QLf6EE/J8GQv88wVkR2toQlM8SWcL+JKUfuGVTBLi32Twcq0B mSs+XffrY1am4Lo9v8E/yLQ1d8ysQWqgd5mb7g+ZbpX4tAdKgobj5JlNLBSDPfpgC8xjUt wod0oY0Ig+o+DaAhougQWFhJz3ptRbE= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=J9ujTFhF; spf=pass (imf29.hostedemail.com: domain of sj@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=sj@kernel.org; dmarc=pass (policy=none) header.from=kernel.org Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 601C560B24; Wed, 30 Aug 2023 01:51:14 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 646B3C433C8; Wed, 30 Aug 2023 01:51:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1693360273; bh=DWBYA7oCvZki07DkGCjwenj2DTnQDU4zQfO0NImpIIo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=J9ujTFhFF4k1ZIMon55gd/JhFNw2FJ9/KoCG5ojGgf5lADVaQLCCPZPgQ31fvsLjB 8eq9CpsL3ulISyFjf1W9BNluqlv165RKtB7SZeBvMYzRiichmuPwe7S567G4mv4r8Q 4/G0Bwtr7W6UfJDzzgobdO+a8zT29CsLy6J4NgdI6XxYZyj+d9j7V6SO5T00TH7ggS O1nX4EAdVY7juvxBBLUU3IOBdxhXqpGFVt3OW6Rad5qcJwb+mmV4S+oVV/mwkbHlJp V/CCOk9iRruU5OvgMvXRd1yQ5etrNWdi8pswZTrAZQAlC36oNUup7/hCRkjXIQcJJx aKe6gcZCrklFg== From: SeongJae Park To: SeongJae Park Cc: damon@lists.linux.dev, Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH] mm/damon/core: use number of passed access sampling as a timer Date: Wed, 30 Aug 2023 01:51:10 +0000 Message-Id: <20230830015110.46420-1-sj@kernel.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20230827003727.49369-1-sj@kernel.org> References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 5F126120009 X-Rspam-User: X-Stat-Signature: br9gtm7yi7sw5h5a5s6aeo5dbxbfd364 X-Rspamd-Server: rspam03 X-HE-Tag: 1693360275-737067 X-HE-Meta: U2FsdGVkX1+5vkqfciVJZ44paJeZPmFc5CmXWW0W6LlUSEB0bx+7efZ7dC7f2Wq88Ji/WvDYENnf1BGpkA7keFNzXJwSnRN2WPr9xkUAiPF+ey5M2K0kQqlQOFi1xqs3Dsn1TncAN9kvr5r/8wWc5x8w6iOTAtRuxUde8L/64kfcR8MIivMc0VXQnoNEsKtyy/xPTCzwHMJPczIhnOhAYP2FzfH9I/bbrMv/DWpq+tGC1kET1a8rgsJ1eYBCrjiRkRX1jlJz4UcnWFhqpK/mMDOHCiF1AysN53IpEN27b248T9F06Mbyi9niJ/rSv0TlTu0MT/6xs2njXWTqOsGdiopfTzu9Q7OJhDIp1smWv71ih5YkDLKzpQ2JToRWlajY+zAKReUcnJYrROYFIISQD3IKCUgAbayeJEoz6SUb3lusYljcW80f8nHCAf2EERhXoOFk3Plezs9TJrS3Z+9IEEOA/LHFjYHZazzNlDGRdkjlPUoPG3FqkTvn+ThWYu7J57ZNjkYFH1fcDTiPTFskjkJe7ohSlaB8yTS4cmDgFlC+lyh2NFU/sVhhepjg7PHYcLDKlctCK+Le7mNEXy4w2nfE/wYOFcTzIeS4nzn0U9ZYk+q+cuOvcyivW8ct3yvgiqJG9ifWcQGOMBs/kmuZchDrFbbalybwoyV2VAe4JMWjVovZQlBdC1H4Yb0L3LlaEFPoWj8EII0vlOWJYG7XboPJ59BFz0GsvX1I8qd+uanlCr3pkpDuwy2R+hefuKgj4q6xdJc+taBZJBmh1JrM1YkXyCRovyb9eH7zJgH6vn72PQvlngouj/Rs2mWx1Of+j+nLV9ABvd35QftUN+CFHo/g9k2LHwxIqsYPHdO2uQadO+SQ9F8EOw5QdEpHssKyDzBsQsyM9sV90yZCkfTpXKPjCqcbs9OVNZssxS15lAQmulhgH6m6p5qCUjE+RnCJaJtrqadt9OVB6HI+4ZY B6nLawZ1 bM3otQwAOrzikdP/kJubNX7b3Tha+8RegwjYZOkc9p4t2jpci6UH2owSiP9IPRpGeKpaZqfPRBTH1HrqLQi1w+VlDxGuqSZw+XrUuuZ5fpfH3U5fyvL9/RDzkMW/duJgIRJ6yxBGAfXk5nuYokOM0S/MRQyV9zhygORkhziu7wtY1LHkMDLBijZnhUaSEQLpeJEduDTMqfYrDtwcr7igdHDOvPOIn7LzfaO0zjJS0bLWAS8QGyy8SB2xU1spmQLHu7Ch93MCebGMB1gp0G6yli3JC6z2xqr+aLPVYKvS7C7ZCIlYZY9fx4HgMvIhMRE1XtJjY96wsVzwJB6ARoqWBvw88Gg== 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: On Sun, 27 Aug 2023 00:37:27 +0000 SeongJae Park wrote: > DAMON sleeps for sampling interval after each sampling, and check if > it's time for further doing aggregation and ops updating using > ktime_get_coarse_ts64() and baseline timestamps for the two periodic > operations. That's for making the operations occur at deterministic > timing. However, it turned out it could still result in indeterministic > and even not-that-intuitive results. > > After all, timer functions, and especially sleep functions that DAMON > uses to wait for specific timing, could contain some errors. Those > errors are legal, so no problem. However, depending on such legal > timing errors, the nr_accesses can be larger than aggregation interval > divided by sampling interval. For example, with the default setting (5 > ms sampling interval and 100 ms aggregation interval) we frequently show > regions having nr_accesses larger than 20. Also, if the execution of a > DAMOS scheme takes a long time, next aggregation could happen before > enough number of samples are collected. > > Since access check sampling is the smallest unit work of DAMON, using > the number of passed sampling intervals as the DAMON-internal timer can > easily avoid these problems. That is, convert aggregation and ops > update intervals to numbers of sampling intervals that need to be passed > before those operations be executed, count the number of passed sampling > intervals, and invoke the operations as soon as the specific amount of > sampling intervals passed. Make the change. > > Signed-off-by: SeongJae Park > --- > include/linux/damon.h | 14 ++++++-- > mm/damon/core.c | 84 +++++++++++++++++++------------------------ > 2 files changed, 48 insertions(+), 50 deletions(-) > > diff --git a/include/linux/damon.h b/include/linux/damon.h > index ab3089de1478..9a32b8fd0bd3 100644 > --- a/include/linux/damon.h > +++ b/include/linux/damon.h > @@ -524,8 +524,18 @@ struct damon_ctx { > struct damon_attrs attrs; > > /* private: internal use only */ > - struct timespec64 last_aggregation; > - struct timespec64 last_ops_update; > + /* number of sample intervals that passed since this context started */ > + unsigned long passed_sample_intervals; > + /* > + * number of sample intervals that should be passed before next > + * aggregation > + */ > + unsigned long next_aggregation_sis; > + /* > + * number of sample intervals that should be passed before next ops > + * update > + */ > + unsigned long next_ops_update_sis; > > /* public: */ > struct task_struct *kdamond; > diff --git a/mm/damon/core.c b/mm/damon/core.c > index 988dc39e44b1..83af336bb0e6 100644 > --- a/mm/damon/core.c > +++ b/mm/damon/core.c > @@ -456,8 +456,11 @@ struct damon_ctx *damon_new_ctx(void) > ctx->attrs.aggr_interval = 100 * 1000; > ctx->attrs.ops_update_interval = 60 * 1000 * 1000; > > - ktime_get_coarse_ts64(&ctx->last_aggregation); > - ctx->last_ops_update = ctx->last_aggregation; > + ctx->passed_sample_intervals = 0; > + ctx->next_aggregation_sis = ctx->attrs.aggr_interval / > + ctx->attrs.sample_interval; > + ctx->next_ops_update_sis = ctx->attrs.ops_update_interval / > + ctx->attrs.sample_interval; > > mutex_init(&ctx->kdamond_lock); > > @@ -577,6 +580,9 @@ static void damon_update_monitoring_results(struct damon_ctx *ctx, > */ > int damon_set_attrs(struct damon_ctx *ctx, struct damon_attrs *attrs) > { > + unsigned long sample_interval; > + unsigned long remaining_interval_us; > + > if (attrs->min_nr_regions < 3) > return -EINVAL; > if (attrs->min_nr_regions > attrs->max_nr_regions) > @@ -584,6 +590,20 @@ int damon_set_attrs(struct damon_ctx *ctx, struct damon_attrs *attrs) > if (attrs->sample_interval > attrs->aggr_interval) > return -EINVAL; > > + sample_interval = attrs->sample_interval ? attrs->sample_interval : 1; > + > + /* adjust next_aggregation_sis */ > + remaining_interval_us = ctx->attrs.sample_interval * > + (ctx->next_aggregation_sis - ctx->passed_sample_intervals); > + ctx->next_aggregation_sis = ctx->passed_sample_intervals + > + remaining_interval_us / sample_interval; > + > + /* adjust next_ops_update_sis */ > + remaining_interval_us = ctx->attrs.sample_interval * > + (ctx->next_ops_update_sis - ctx->passed_sample_intervals); > + ctx->next_ops_update_sis = ctx->passed_sample_intervals + > + remaining_interval_us / sample_interval; > + > damon_update_monitoring_results(ctx, attrs); > ctx->attrs = *attrs; > return 0; > @@ -757,38 +777,6 @@ int damon_stop(struct damon_ctx **ctxs, int nr_ctxs) > return err; > } So, the new timer variables are initialized in damon_new_ctx() as the default (5ms sampling interval, 100ms aggregation interval, and 1s ops update interval) and adjusted in damon_set_attrs(). It means the first intervals will be the default ones always. I will make those initialized at the beginning of kdamond_fn() from the next spin. [...] > @@ -1436,6 +1412,8 @@ static int kdamond_fn(void *data) > sz_limit = damon_region_sz_limit(ctx); As mentioned abovely, the timer variables will be initialized around here (at the beggining of kdamond_fn(), before going into the loop), in the next spin. Thanks, SJ > > while (!kdamond_need_stop(ctx)) { > + unsigned long sample_interval; > + > if (kdamond_wait_activation(ctx)) > break; > > @@ -1446,11 +1424,17 @@ static int kdamond_fn(void *data) > break; > > kdamond_usleep(ctx->attrs.sample_interval); > + ctx->passed_sample_intervals++; > > if (ctx->ops.check_accesses) > max_nr_accesses = ctx->ops.check_accesses(ctx); > > - if (kdamond_aggregate_interval_passed(ctx)) { > + sample_interval = ctx->attrs.sample_interval ? > + ctx->attrs.sample_interval : 1; > + if (ctx->passed_sample_intervals == > + ctx->next_aggregation_sis) { > + ctx->next_aggregation_sis += > + ctx->attrs.aggr_interval / sample_interval; > kdamond_merge_regions(ctx, > max_nr_accesses / 10, > sz_limit); > @@ -1465,7 +1449,11 @@ static int kdamond_fn(void *data) > ctx->ops.reset_aggregated(ctx); > } > > - if (kdamond_need_update_operations(ctx)) { > + if (ctx->passed_sample_intervals == > + ctx->next_ops_update_sis) { > + ctx->next_ops_update_sis += > + ctx->attrs.ops_update_interval / > + sample_interval; > if (ctx->ops.update) > ctx->ops.update(ctx); > sz_limit = damon_region_sz_limit(ctx); > -- > 2.25.1 > >