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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 5614AC3DA63 for ; Wed, 24 Jul 2024 14:42:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=MFRmEFHY//Qp9lgUeczFB5mb8XNPQk1Nd66DBnuGydc=; b=Bw0QPvk3MNPlglOfmpFl7aW07z QbnyOBi8F/LOJXGU4BfcgI4bi12ie9D9pN9Va4GnvSEvOP6vVcSEajNOaxHEs2gkXA8cmNKMnBEc5 jX8PsZzZRuf67/dMhlE3LRiOx/LU9HU8uAbK1u52hJL+xHnQW58YHvVRHtLivz6RCYQGthlGKGhrK 2TeSzZtkpjrxdFXIBUZD4rPRD2W12GK5c9Z9Wbi7OjbVgdg2B4fD6KJP4RAQiBs+OJWcd3cq079t0 QIB6IwVWUcnfXvrmYLntJ+OsJsjloABC1Vy84IWr19YSgDZpmi1RruXePn2sLZvTVpcC28Eaoermd 6FhmjFSw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sWdBb-0000000Fdgm-3xnI; Wed, 24 Jul 2024 14:41:48 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sWdBB-0000000FdWv-0CUF for linux-arm-kernel@lists.infradead.org; Wed, 24 Jul 2024 14:41:23 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 95FF8106F; Wed, 24 Jul 2024 07:41:45 -0700 (PDT) Received: from pluto (usa-sjc-mx-foss1.foss.arm.com [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C276C3F5A1; Wed, 24 Jul 2024 07:41:18 -0700 (PDT) Date: Wed, 24 Jul 2024 15:41:15 +0100 From: Cristian Marussi To: Luke Parkin Cc: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, arm-scmi@vger.kernel.org, sudeep.holla@arm.com, cristian.marussi@arm.com Subject: Re: [PATCH v3 5/5] firmware: arm_scmi: Reset statistics Message-ID: References: <20240715133751.2877197-1-luke.parkin@arm.com> <20240715133751.2877197-6-luke.parkin@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240715133751.2877197-6-luke.parkin@arm.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240724_074121_235872_9A2297CD X-CRM114-Status: GOOD ( 22.42 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Jul 15, 2024 at 02:37:51PM +0100, Luke Parkin wrote: > Create write function to reset individual statistics on write > Create reset_all stats debugfs file to reset all statistics > > Signed-off-by: Luke Parkin > --- > drivers/firmware/arm_scmi/driver.c | 104 +++++++++++++++++++++-------- > 1 file changed, 78 insertions(+), 26 deletions(-) > > diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c > index 9378e2d8af4f..6a90311f764d 100644 > --- a/drivers/firmware/arm_scmi/driver.c > +++ b/drivers/firmware/arm_scmi/driver.c > @@ -2865,6 +2865,47 @@ static int scmi_device_request_notifier(struct notifier_block *nb, > return NOTIFY_OK; > } > > +static int read_atomic(void *atomic, u64 *val) > +{ > + atomic_t *atm = (atomic_t *)atomic; > + > + *val = atomic_read(atm); > + return 0; > +} > + > +static int reset_single(void *atomic, u64 val) > +{ > + atomic_t *atm = (atomic_t *)atomic; > + > + atomic_set(atm, 0); > + return 0; > +} > + So, you rightly built a fops_reset_on_write to handle any write to a single stats field and zero teh counter...BUT...for the sake of simplicity, we could relax a bit the requirement and just think about handling generically the writes...because if you look at the definition of debugfs_create_atomic_t: https://elixir.bootlin.com/linux/v6.10/source/fs/debugfs/file.c#L867 you will see that if you declare the mode as RW 0600, the debugfs core code will automagically provide you with RW debugfs atomic fops...so that you wont need all of this and you can still keep using debugfs_create_atomic_t....the only limitation is that the user will be able to reset the counter to any value, NOT only to zero...BUT being a debug/test feats seems to me acceptable. > +static void reset_all_stats(struct scmi_info *info) > +{ > + for (int i = 0; i < LAST; i++) > + atomic_set(&info->dbg_stats[i], 0); > +} You can drop this function and just nest the for loop inside the function below... > + > +static ssize_t reset_all_on_write(struct file *filp, > + const char __user *buf, > + size_t count, loff_t *ppos) > +{ > + struct scmi_info *info = filp->private_data; > + > + reset_all_stats(info); > + return count; > +} > + > +DEFINE_DEBUGFS_ATTRIBUTE(fops_reset_on_write, read_atomic, reset_single, "%llu\n"); > + ..and drop this as said above to keep using debugfs_create_atomic_t > +static const struct file_operations fops_reset_stats = { > + .owner = THIS_MODULE, > + .open = simple_open, > + .llseek = no_llseek, > + .write = reset_all_on_write, > +}; > + This is good instead for a quick general reset... > static void scmi_debugfs_stats_setup(struct scmi_info *info, > struct dentry *trans) > { > @@ -2872,32 +2913,43 @@ static void scmi_debugfs_stats_setup(struct scmi_info *info, > > stats = debugfs_create_dir("stats", trans); > > - debugfs_create_atomic_t("sent_ok", 0400, stats, using 0600 here made the trick... > - &info->dbg_stats[SENT_OK]); Thanks, Cristian