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 063C2C71153 for ; Mon, 11 Sep 2023 19:05:13 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 334BB6B02D4; Mon, 11 Sep 2023 15:05:13 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2BDFE6B02D8; Mon, 11 Sep 2023 15:05:13 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 137DB6B02D9; Mon, 11 Sep 2023 15:05:13 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id F2DB26B02D4 for ; Mon, 11 Sep 2023 15:05:12 -0400 (EDT) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id B644E80AE9 for ; Mon, 11 Sep 2023 19:05:12 +0000 (UTC) X-FDA: 81225244464.23.40DBCE6 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by imf30.hostedemail.com (Postfix) with ESMTP id A581A80026 for ; Mon, 11 Sep 2023 19:05:10 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=caoz1Q8j; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf30.hostedemail.com: domain of sj@kernel.org designates 145.40.68.75 as permitted sender) smtp.mailfrom=sj@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1694459111; 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=FWGqSIaB6S6ZrwTQ+KViAB+fVTEwMAyzWl7opxwuQas=; b=7SlN+mTe+EzxMWaC5F5+40weULNqhIlfmNN7SJ4psIZIDiet3vA1HvmLsiRnT2zTvWcY16 Zx/F5SLFY1/oQ6dab36iONRvncYu1HZk67LdUddPLVbMcwWOHW4iDvoHxAXGK5dreNFXNa doauWskEkwhSHd8nLFbaRrII7Q8hA/0= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=caoz1Q8j; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf30.hostedemail.com: domain of sj@kernel.org designates 145.40.68.75 as permitted sender) smtp.mailfrom=sj@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1694459111; a=rsa-sha256; cv=none; b=vhXPKH7G/4053mc28NP1EXnRSFLlJ6iKnHgG1bEQgWPMsOJX9uVdcvkX8oTgLIASDqxd/G q2nTvdGEePN2vzSOktuoAp4h6TDvWD61lhH/mtJo59Jf75ul33z7dhHTC8czq7rqFSr4JQ uH/BUFSLRaFw0CG85TpzTZXFBupvwAY= 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 ams.source.kernel.org (Postfix) with ESMTPS id 35C4FB81760; Mon, 11 Sep 2023 19:05:08 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id F3F44C433C8; Mon, 11 Sep 2023 19:05:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1694459106; bh=hJ5p3TsjWyGe3OdxMfLULkedWZ9EMmXGZBw1r4Vi72Y=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=caoz1Q8jo/vriTwrAeUv58MBLku2nEOL8/UhVo7wZQGVqe4qs9eGOvszaUThBXO8K eygwuQv3xjXGPgYCHZlTEyi+gXH2EMDecgQRSPLCiZVOxi/YPDCMZmHhaJoTLN25E4 7B5acEhPsH1lGAKHbQ1XTPI/V6tTPkY6tLjcvgJR9cTcvsP6Acod9lQpWMIwwXJxpm WoUhmaub8YtJdN2HGMzGE/a73XBz962VwFBy6eUxdnHSlTBRwXVT6QCt3Y65BLXb2A AGQkYxO/1MVwyAn0nED4/6FNf/obpa58p85D6mmq8eV0KHbg8ARSa829KdYdzmCjEG RbqIJxQvZ9reQ== From: SeongJae Park To: Steven Rostedt Cc: SeongJae Park , Andrew Morton , damon@lists.linux.dev, linux-mm@kvack.org, linux-trace-kernel@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/2] mm/damon/core: add a tracepoint for damos apply target regions Date: Mon, 11 Sep 2023 19:05:04 +0000 Message-Id: <20230911190504.102317-1-sj@kernel.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20230911141955.245d1397@gandalf.local.home> References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: A581A80026 X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: g43pzzfzmgso4zfm39a4e6oewxdohg67 X-HE-Tag: 1694459110-971064 X-HE-Meta: U2FsdGVkX1/tzRnriYRjRIBBnxmb0LKckwjYb4Am53e2510cem5MkzT5bnfhoAwOPMjaieqwVHKV0ckSCFC5sHYU9SdOoccBQSHQGkXIE46DMJBLDX2PTzzY1Z870wjPTC7n4dj9XJkblwLsVw6VRjABOG4UIL0R6EhzPv17x3iqc8Q8U1ZmdU/eYYJxCnJKBdcx81YV+qbaao5Id3z/Hi60k0oMdAL3h1bnUdlrIccN9M9fj/VIewsHw2gqNLoqRMeCaawSKaF0MVQ7oXIyIX88VycowxkacmQRPCdAkTBSDnD/7sZ5oRP/XqrWPopOg0H4xP6xPKWYezAIn9GTJQ8PsNlnMtRUsl5WT/7xaXJ131VmwFw5KZsVyWlDulvt2Y8dPhw584tSCrrVK3sXFsXe1+7JxTCPIM1ByOJRdY4DRSPkWqRK6K8JwFAFWNnkaE9GIu3ceg5rotI5BdnrVdrIdyqe38KZZ2PEghc6L7YqUzRP5rJLgrzDk5wxUBSV1eKv5N3z4Ycf65zDKgAoftU6vQX65IZ/cAWooUN09YlYnMgWxWcXjA1LMA/+4hwxQyhhv/A0RhYHbR8bQZfkftvA72eBflmSzg49MR0RrhfmjJdi5jA/BMmMLoBVuI8JjzcVsw5ATeY6b5tNJP1ZAPtVT0z8IptiEiCmfVX/KW2GD6LryGMEgafx//6ZDshcDF+1Sij5KbF84Q/lOJz8uRQb4HGaUpsvXuH0rQ77Sjga8j3y9q85gH/Vgm9VlvGPkS/+CF9YqAtKJ5jFsueMwNKnzbrX++tnw2VSce9kZ00658Z5hFGFkCIEcJXB/b+LHM/og5OAwWO8GPEk7Oxkzw52+qGYRjDAIdWLbLq0dL+StvRzAX8B/A9gDW1P80KoclbEwfo8cbgDk1IZmsVqnS/YQLXA1SHn+IKnty/OJ3Bx8uYX3IJEh6cl/2TMFA0H0Q43fTD+mg+ywR4+xLC pGCXEEjD X7NXu0yE8QqtRJ9QjYDSwBsl4r5zIgSOOe8Yw8peVmbBWQnFFjqgH7e7JBvoUPfQPpsRdCErEK6S9AmkgnFrhL6dPTtdLpZ51w+HIvwdu5GlNme+KlECUJtQ4hMOcYuZUKZ7CDCRTI6c0u/nEilTUZBW4zYYa1q4XlWO9Qj0JB0qnWkqLpxSTklBjZDIC9FmeTD5Knj1jIaNtAIs1zi/mO3vkpuJevaa5PWVxKbBYy1w7+79vohuRl6pD+qKYGoPhcojyHAGUwHGGIPmA8wEnx2twYohPYb8ITJS17VCsdqBYOk8L6wL4v1x5hu71cQjp5q+FPxOwsduh7I/k9WuTRRIJhgzA7wTV4wgKKDrhE2uqPw4QZJJ43s60mXywtC+kZQm5WYHBPohBBxo= 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: Hi Steven, On Mon, 11 Sep 2023 14:19:55 -0400 Steven Rostedt wrote: > On Mon, 11 Sep 2023 04:59:07 +0000 > SeongJae Park wrote: > > > --- a/mm/damon/core.c > > +++ b/mm/damon/core.c > > @@ -950,6 +950,28 @@ static void damos_apply_scheme(struct damon_ctx *c, struct damon_target *t, > > struct timespec64 begin, end; > > unsigned long sz_applied = 0; > > int err = 0; > > + /* > > + * We plan to support multiple context per kdamond, as DAMON sysfs > > + * implies with 'nr_contexts' file. Nevertheless, only single context > > + * per kdamond is supported for now. So, we can simply use '0' context > > + * index here. > > + */ > > + unsigned int cidx = 0; > > + struct damos *siter; /* schemes iterator */ > > + unsigned int sidx = 0; > > + struct damon_target *titer; /* targets iterator */ > > + unsigned int tidx = 0; > > + > > If this loop is only for passing sidx and tidx to the trace point, You're correct. > you can add around it: > > if (trace_damos_before_apply_enabled()) { > > > + damon_for_each_scheme(siter, c) { > > + if (siter == s) > > + break; > > + sidx++; > > + } > > + damon_for_each_target(titer, c) { > > + if (titer == t) > > + break; > > + tidx++; > > + } > > } > > > And then this loop will only be done if that trace event is enabled. Today I learned yet another great feature of the tracing framework. Thank you Steven, I will add that to the next spin of this patchset! > > To prevent races, you may also want to add a third parameter, or initialize > them to -1: > > sidx = -1; > > if (trace_damo_before_apply_enabled()) { > sidx = 0; > [..] > } > > And you can change the TRACE_EVENT() TO TRACE_EVENT_CONDITION(): > > TRACE_EVENT_CONDITION(damos_before_apply, > > TP_PROTO(...), > > TP_ARGS(...), > > TP_CONDITION(sidx >= 0), > > and the trace event will not be called if sidx is less than zero. > > Also, this if statement is only done when the trace event is enabled, so > it's equivalent to: > > if (trace_damos_before_apply_enabled()) { > if (sdx >= 0) > trace_damos_before_apply(cidx, sidx, tidx, r, > damon_nr_regions(t)); > } Again, thank you very much for letting me know this awesome feature. However, sidx is supposed to be always >=0 here, since kdamond is running in single thread and hence no race is expected. If it exists, it's a bug. So, I wouldn't make this change. Appreciate again for letting me know this very useful feature, and please let me know if I'm missing something, though! Thanks, SJ > > -- Steve > > > > > > > if (c->ops.apply_scheme) { > > if (quota->esz && quota->charged_sz + sz > quota->esz) { > > @@ -964,8 +986,11 @@ static void damos_apply_scheme(struct damon_ctx *c, struct damon_target *t, > > ktime_get_coarse_ts64(&begin); > > if (c->callback.before_damos_apply) > > err = c->callback.before_damos_apply(c, t, r, s); > > - if (!err) > > + if (!err) { > > + trace_damos_before_apply(cidx, sidx, tidx, r, > > + damon_nr_regions(t)); > > sz_applied = c->ops.apply_scheme(c, t, r, s); > > + } > > ktime_get_coarse_ts64(&end); > > quota->total_charged_ns += timespec64_to_ns(&end) - > > timespec64_to_ns(&begin); > > -- >