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 5AB27C433F5 for ; Wed, 8 Dec 2021 16:49:11 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 76DD66B0072; Wed, 8 Dec 2021 11:49:00 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 71D756B0073; Wed, 8 Dec 2021 11:49:00 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5E5396B0074; Wed, 8 Dec 2021 11:49:00 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0225.hostedemail.com [216.40.44.225]) by kanga.kvack.org (Postfix) with ESMTP id 50DB46B0072 for ; Wed, 8 Dec 2021 11:49:00 -0500 (EST) Received: from smtpin17.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 1A02F32619 for ; Wed, 8 Dec 2021 16:48:50 +0000 (UTC) X-FDA: 78895211220.17.632D576 Received: from sin.source.kernel.org (sin.source.kernel.org [145.40.73.55]) by imf04.hostedemail.com (Postfix) with ESMTP id 19B1440002 for ; Wed, 8 Dec 2021 16:48:48 +0000 (UTC) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sin.source.kernel.org (Postfix) with ESMTPS id 7D609CE225F; Wed, 8 Dec 2021 16:48:44 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0EA9DC341C7; Wed, 8 Dec 2021 16:48:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1638982122; bh=qMVPh2Qo//JpTSUpFuDDiF5JmeH3DO3HcB3Rv4RljOk=; h=From:To:Cc:Subject:Date:In-Reply-To:From; b=VZhAHiuk9I8X4JhBsE759+/S1OX7qdJQsYvFYy7RcyvJET4jNqc1ZZIcberY204ei WZ3Q3mPE3mf8x0j8eH/N9vEhDpXj1djg8gTjVA2MYRhnxNtuuvTpLlCw4UQpH4ruE+ I/jSWCgMFFJFSI8e7F9ihRZ2QGJM++tCQ+Ttc8hyu6Xu134lzoUv3/ltvBcUgZnBXi B3luttQjGi9G9Km/wz8phl+NswFR6JcJgQbDd6UXosyYupx2NW0nQEHUy5xBRw1A3B ik9hfFUA6FvQNJa8Vq5XC5FwU/h6wCpGO9kGqQ8iQjipM/HtRD1Io9Dt/Bx29HkbBO yBGZUKnPJTzqw== From: SeongJae Park To: Xin Hao Cc: SeongJae Park , akpm@linux-foundation.org, shuah@kernel.org, brendanhiggins@google.com, linux-mm@kvack.org, linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 02/11] mm/damon/dbgfs: Remove an unnecessary error message Date: Wed, 8 Dec 2021 16:48:38 +0000 Message-Id: <20211208164838.34177-1-sj@kernel.org> X-Mailer: git-send-email 2.17.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=UTF-8 X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 19B1440002 X-Stat-Signature: y86xbzkzppp3pqb783i6no48sy3wjg94 Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=VZhAHiuk; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf04.hostedemail.com: domain of sj@kernel.org designates 145.40.73.55 as permitted sender) smtp.mailfrom=sj@kernel.org X-HE-Tag: 1638982128-610983 Content-Transfer-Encoding: quoted-printable 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 Wed, 8 Dec 2021 23:13:34 +0800 Xin Hao wrote: > Hi SeongJae: >=20 > On 12/8/21 8:49 PM, SeongJae Park wrote: > > On Wed, 8 Dec 2021 14:29:40 +0800 Xin Hao wr= ote: > > > > Hi Xin, > > > >> Hi park: > >> > >> On 12/1/21 11:04 PM, SeongJae Park wrote: > >>> When wrong scheme action is requested via the debugfs interface, DA= MON > >>> prints an error message. Because the function returns error code, = this > >>> is not really needed. Because the code path is triggered by the us= er > >>> specified input, this can result in kernel log mistakenly being mes= sy. > >> Completely correct, but there will also be a problem that users can=E2= =80=99t > >> quickly locate where the problem is, > >> > >> Especially too many parameters need to be written into the interface= . > >> > >> I think it is necessary to add some debugging methods to help users = find > >> the error without polluting the kernel log. > >> > >> And i have an idea, like this: > >> > >> in dbgfs, add a last_cmd_stat interface. > >> > >> # echo "1 2 1 2 1 2 1 2 1 2 100 ..." > schemes > >> > >> # cat last_cmd_stat > >> > >> # wrong action 100 > >> > >> In this way, on the one hand, it will not pollute the kernel log, on= the > >> other hand, it will help users find the cause of the operation > >> interface error. > >> > >> Park, how do you think of about this idea, if ok, i will send a patc= h. > > Thank you always for your great suggestions and efforts! BTW, I pref= er to be > > called with my first name ;) > Ha-Ha, Sorry! > > > > I want DAMON kernel code to be as simple and small as possible, while= putting > > fancy but complicated features for user conveniences in user space to= ols like > > DAMO[1]. In other words, I hope the DAMON debugfs interface to be us= ed as an > > interface for such user space tools, not an interface for human hands= . > Ok, I know what you mean. > > > > IMHO, implementing the feature you proposed in the kernel could make = the code > > slightly bigger, while it can easily implemented in user space. I th= erefore > > think the feature would be better to be implemented in user space. I= f you > > could send a pull request of the feature for DAMO, it would be so gre= at. >=20 > Ok, i will do it, But there's a problem here, If the user does not us= e=20 > the DAMO tools to operate the dbgfs interface, Well, I don't think that as a problem, but a room for improvement. Maybe= we could improve the documentation. Thanks, SJ >=20 > the operation interface error will still hard to find the cause of erro= rs. >=20 >=20 > > > > [1] https://github.com/awslabs/damo > > > > > > Thanks, > > SJ > > > >>> To avoid the case, this commit removes the message > >>> > >>> Fixes: af122dd8f3c0 ("mm/damon/dbgfs: support DAMON-based Operation= Schemes") > >>> Signed-off-by: SeongJae Park > >>> --- > >>> mm/damon/dbgfs.c | 4 +--- > >>> 1 file changed, 1 insertion(+), 3 deletions(-) > >>> > >>> diff --git a/mm/damon/dbgfs.c b/mm/damon/dbgfs.c > >>> index 4bf4204444ab..5b628990ae6e 100644 > >>> --- a/mm/damon/dbgfs.c > >>> +++ b/mm/damon/dbgfs.c > >>> @@ -210,10 +210,8 @@ static struct damos **str_to_schemes(const cha= r *str, ssize_t len, > >>> &wmarks.low, &parsed); > >>> if (ret !=3D 18) > >>> break; > >>> - if (!damos_action_valid(action)) { > >>> - pr_err("wrong action %d\n", action); > >>> + if (!damos_action_valid(action)) > >>> goto fail; > >>> - } > >>> =20 > >>> if (min_sz > max_sz || min_nr_a > max_nr_a || min_age > max_ag= e) > >>> goto fail; > >> --=20 > >> Best Regards! > >> Xin Hao > >> > --=20 > Best Regards! > Xin Hao