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 4DFC4C433FE for ; Wed, 8 Dec 2021 15:14:11 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8020F6B0072; Wed, 8 Dec 2021 10:14:00 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 7B15B6B0073; Wed, 8 Dec 2021 10:14:00 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6A1AD6B0074; Wed, 8 Dec 2021 10:14:00 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (relay028.a.hostedemail.com [64.99.140.28]) by kanga.kvack.org (Postfix) with ESMTP id 5CDF86B0072 for ; Wed, 8 Dec 2021 10:14:00 -0500 (EST) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay12.hostedemail.com (Postfix) with ESMTP id 1BB65120BBC for ; Wed, 8 Dec 2021 15:13:43 +0000 (UTC) X-FDA: 78894971526.23.9864B4E Received: from out30-43.freemail.mail.aliyun.com (out30-43.freemail.mail.aliyun.com [115.124.30.43]) by imf14.hostedemail.com (Postfix) with ESMTP id CC67E6001993 for ; Wed, 8 Dec 2021 15:13:40 +0000 (UTC) X-Alimail-AntiSpam:AC=PASS;BC=-1|-1;BR=01201311R121e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=e01e04357;MF=xhao@linux.alibaba.com;NM=1;PH=DS;RN=7;SR=0;TI=SMTPD_---0UzxF8cs_1638976414; Received: from B-X3VXMD6M-2058.lan(mailfrom:xhao@linux.alibaba.com fp:SMTPD_---0UzxF8cs_1638976414) by smtp.aliyun-inc.com(127.0.0.1); Wed, 08 Dec 2021 23:13:36 +0800 From: Xin Hao Reply-To: xhao@linux.alibaba.com Subject: Re: [PATCH 02/11] mm/damon/dbgfs: Remove an unnecessary error message To: SeongJae Park Cc: akpm@linux-foundation.org, shuah@kernel.org, brendanhiggins@google.com, linux-mm@kvack.org, linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org References: <20211208124938.4035-1-sj@kernel.org> Message-ID: Date: Wed, 8 Dec 2021 23:13:34 +0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.14.0 MIME-Version: 1.0 In-Reply-To: <20211208124938.4035-1-sj@kernel.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: CC67E6001993 X-Stat-Signature: f45w5zynxa58aua3t9m1qn8tf9bgk55b Authentication-Results: imf14.hostedemail.com; dkim=none; spf=pass (imf14.hostedemail.com: domain of xhao@linux.alibaba.com designates 115.124.30.43 as permitted sender) smtp.mailfrom=xhao@linux.alibaba.com; dmarc=pass (policy=none) header.from=alibaba.com X-HE-Tag: 1638976420-778065 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 SeongJae: On 12/8/21 8:49 PM, SeongJae Park wrote: > On Wed, 8 Dec 2021 14:29:40 +0800 Xin Hao wrote: > > Hi Xin, > >> Hi park: >> >> On 12/1/21 11:04 PM, SeongJae Park wrote: >>> When wrong scheme action is requested via the debugfs interface, DAMON >>> prints an error message. Because the function returns error code, this >>> is not really needed. Because the code path is triggered by the user >>> specified input, this can result in kernel log mistakenly being messy. >> Completely correct, but there will also be a problem that users can’t >> 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 patch. > Thank you always for your great suggestions and efforts! BTW, I prefer 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 tools like > DAMO[1]. In other words, I hope the DAMON debugfs interface to be used 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 therefore > think the feature would be better to be implemented in user space. If you > could send a pull request of the feature for DAMO, it would be so great. Ok,  i will do it,  But there's a problem here, If the user does not use the DAMO tools to operate  the dbgfs interface, the operation interface error will still hard to find the cause of errors. > > [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 char *str, ssize_t len, >>> &wmarks.low, &parsed); >>> if (ret != 18) >>> break; >>> - if (!damos_action_valid(action)) { >>> - pr_err("wrong action %d\n", action); >>> + if (!damos_action_valid(action)) >>> goto fail; >>> - } >>> >>> if (min_sz > max_sz || min_nr_a > max_nr_a || min_age > max_age) >>> goto fail; >> -- >> Best Regards! >> Xin Hao >> -- Best Regards! Xin Hao