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 4E066C7115A for ; Sun, 22 Jun 2025 16:14:30 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A164B6B0096; Sun, 22 Jun 2025 12:14:29 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9C6AF6B0099; Sun, 22 Jun 2025 12:14:29 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8DC6E6B009B; Sun, 22 Jun 2025 12:14:29 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 811D06B0096 for ; Sun, 22 Jun 2025 12:14:29 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 241A9BC87A for ; Sun, 22 Jun 2025 16:14:29 +0000 (UTC) X-FDA: 83583534258.13.FBA17B3 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf04.hostedemail.com (Postfix) with ESMTP id 73DF040009 for ; Sun, 22 Jun 2025 16:14:27 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=gXgM1wZU; spf=pass (imf04.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=1750608867; 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=45p03UdXDkULn8sBRfnuJox0eZRkgaQ7ad9GSBbF0/Y=; b=zIkgCZavFyEWmDr4GUF20PPFc981+IMXi2Mu7CXI4YD77PH/Sz37Cipv2hVfbEGty9EEdM aIfsHE2N9yImxgD7qLZgbeCUdsxZngCtNPvbtMa61WjxrVm8IdtnN2asLCsWtqrnOfLlzF WiFD+/bukAGrmqlxAq5UFUr+za7Lm4I= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=gXgM1wZU; spf=pass (imf04.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=1750608867; a=rsa-sha256; cv=none; b=Zuk6e09aJCWvuUuSNFvOmk8uRIFmvQViWGTXsDmNzYGoAxgZ+2lykBBi+xprsIQ9eFd5zA QoDUf856g/OmpJjxu2mT1cEJqGIPT2AAZYQazFq0nejo+kqdDsVP2nGTF/0T1SSMWbXVzl mDmY2VHUF0IkaG9aXhPFcEN1UthVKro= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 1C2BC5C49D3; Sun, 22 Jun 2025 16:12:10 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 05121C4CEF1; Sun, 22 Jun 2025 16:14:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1750608866; bh=4pHRnQJXUfptxQxwq1SvF+KeLINCGhxIhnKd3EcOKwM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=gXgM1wZUQQNCWZaLZ26CgqvfdJZ/yzzTrW2KeqDVsDJ8+9uwz/V6LZyetWvAOTAX5 OtuWujmhSj3EowCnYQR9FC1reQWzjub/+hFrW2Nu5frL0OBUQX8cR2bRh+ZYI8Bk1a Kl8jp0JU0Yog/KGugbHmPRRosoiMB6NUVucG4ix3khUI/ROzB/od9+HAefJIjWiH0q nMevsJwc3nFNFMhsodQxh2trESKIId7sqcB1lQn0oabHxbknkxL3y0x1hCtmo6BGtR 57mpPmdRlxrYTOPgwN3TUL+6piDrMxtW+1f3JMp92i4VRh6QnDYSW+rntx9GM5V2vC YR/QZwYE5oYsg== From: SeongJae Park To: Honggyu Kim Cc: SeongJae Park , damon@lists.linux.dev, Andrew Morton , linux-mm@kvack.org, kernel_team@skhynix.com, Yunjeong Mun Subject: Re: [PATCH 2/3] samples/damon: change enable parameters to enabled Date: Sun, 22 Jun 2025 09:14:22 -0700 Message-Id: <20250622161422.50852-1-sj@kernel.org> X-Mailer: git-send-email 2.39.5 In-Reply-To: <20250622120926.1712-3-honggyu.kim@sk.com> References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Rspam-User: X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 73DF040009 X-Stat-Signature: znnky9q86bj776u79ctq1ujusxyc5b5u X-HE-Tag: 1750608867-497490 X-HE-Meta: U2FsdGVkX18vxMxiVJQf2PSVijS2W+UlZ+0rU7AvGPAMbiUUTtlhzYXqgq4G9NiIlw9H5ArfWsXlyEC1byUANLI1br39s5hiKcXSU6HTONQkW9piMfhtnN2pWeUzF5kMsZ64Gan5ZENR5cdziSo0O5a9bN2JvWYaN8Es4xDeBecvMgnsATYZAJI8W2bbBHpmrJ+zF5AkRuIc2h7TOWOQTmPeG5YX8sgHRu5ZXfno5z0Q46WSFgRwUh3yWBz5sck1kaJ/kBfQNseknrowU/0rdN8bi6RIEyo+LzDS1ig1stX7BhXrkhrIilALh3nLCh6D5zDzfE//4N1jplLZZ9Csd2MybwzMQlYNKpGhLwMgd1juN313T0xvfvf10IDskh859uo8cMn+49P/E7OFkXfl/7WUU7BEBi2WrdUOZpD396TQgNeaDcx4q6hZGUnpEV1YGpXemLgn2CELWO7NDW/Sl6zsd0ZtCq+xbdJ93zkn+aw8Zul5xdtVp78wc8sgf2veXAqH4WCd0GKe6jB0cWMqEHUF8NMbrJDKif7G9Y44+n+L9QFudQDZo+HEZR0RRygG9il24NVgMNHQG8EEV+v4C/UgYntJuw3DW9xtQwdZou4IN8Iwv3IIpxzzR2FoaGyVG2GEa3FtCdf18bzL1BissIOJq3AKpV4+OFscz2XrWzsxiR5sFc3uRQoBdvxVYLxGLOJFcTrV2ajppi5fEyzxutXJ493e08MSsPnAurHdngjTnVAe9nKTZwxrnw8fq7wSAgfYFnpECt/JECN50aPryXtyxbutn7Vik2ugo/dcBcjMfw73d8lljwRZh4wqwPgEh8F7YSRP1LcdEQPGt9wj4nGtNhtIAfEcKMw+5WMtS22X8w04vXeZeLgc9n7SuT0N6tBsEUUqGnQozFmPjVat889j5RTt6JI8R7MkF4TXHBk7/9Vao8CCy1ONBHGUJ2U2CxLG+m7mUQmHefYDNq6 Vmbz8bP/ jgtR7gt+rMiKnRnY1rTGTyA3vKVMltmL3k5kBtcnDXFBbeyM1JCgLrMssruWcamhq8tp+bAtewXqnkwjqcWvCnpgAX2n3GKkM0le7gEDlXq15rQvPHppP2eGtbBH775iIE9rEiMHjex5vdXu4YfoIOJ668Z7UB2ZFOjerGtCOhC1LwlY+/6l2KH9qMPK7Wm/849W7rvZn9XEPkfaNjK8qmV3nYWBp/xO4GjGr1379YksaJmL5UY+xznxGYSV/x8o+fv4YKDCKU5A+aTO132lp65ITnNQz2gHM/EVL 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: Hi Honggyu, On Sun, 22 Jun 2025 21:09:24 +0900 Honggyu Kim wrote: > The damon_{lru_sort,reclaim,stat} kernel modules use "enabled" > parameter knobs as follows. > > /sys/module/damon_lru_sort/parameters/enabled > /sys/module/damon_reclaim/parameters/enabled > /sys/module/damon_stat/parameters/enabled > > However, other sample modules of damon use "enable" parameter knobs so > it'd be better to rename them from "enable" to "enabled" to keep the > consistency with other damon modules. I think this is a good change, thank you! [...] > > Signed-off-by: Honggyu Kim > Cc: Yunjeong Mun I have simple comments about variable names below. If you address it, please feel free to add below. Reviewed-by: SeongJae Park > --- > samples/damon/mtier.c | 16 ++++++++-------- > samples/damon/prcl.c | 16 ++++++++-------- > samples/damon/wsse.c | 16 ++++++++-------- > 3 files changed, 24 insertions(+), 24 deletions(-) > > diff --git a/samples/damon/mtier.c b/samples/damon/mtier.c > index f3220d6e6739..cb273bf4b5c2 100644 > --- a/samples/damon/mtier.c > +++ b/samples/damon/mtier.c > @@ -33,14 +33,14 @@ module_param(node0_mem_free_bp, ulong, 0600); > static int damon_sample_mtier_enable_store( > const char *val, const struct kernel_param *kp); > > -static const struct kernel_param_ops enable_param_ops = { > +static const struct kernel_param_ops enabled_param_ops = { > .set = damon_sample_mtier_enable_store, > .get = param_get_bool, > }; > > -static bool enable __read_mostly; > -module_param_cb(enable, &enable_param_ops, &enable, 0600); > -MODULE_PARM_DESC(enable, "Enable of disable DAMON_SAMPLE_MTIER"); > +static bool enabled __read_mostly; > +module_param_cb(enabled, &enabled_param_ops, &enabled, 0600); > +MODULE_PARM_DESC(enabled, "Enable or disable DAMON_SAMPLE_MTIER"); > > static struct damon_ctx *ctxs[2]; > > @@ -160,17 +160,17 @@ static void damon_sample_mtier_stop(void) > static int damon_sample_mtier_enable_store( > const char *val, const struct kernel_param *kp) > { > - bool enabled = enable; > + bool enable = enabled; The local variable is to cache the old state of the parameter. Hence I think 'enable' is not a very good name here. I'd prefer naming the local variable and similar ones on other sample modules as 'is_enabled' or somewhat else that better represents its usage. [...] > diff --git a/samples/damon/prcl.c b/samples/damon/prcl.c [...] > @@ -112,17 +112,17 @@ static void damon_sample_prcl_stop(void) > static int damon_sample_prcl_enable_store( > const char *val, const struct kernel_param *kp) > { > - bool enabled = enable; > + bool enable = enabled; Ditto. [...] > diff --git a/samples/damon/wsse.c b/samples/damon/wsse.c [...] > @@ -92,17 +92,17 @@ static void damon_sample_wsse_stop(void) > static int damon_sample_wsse_enable_store( > const char *val, const struct kernel_param *kp) > { > - bool enabled = enable; > + bool enable = enabled; Here, too. [...] Thanks, SJ