linux-kernel - Re: [PATCH v2 4/4] firmware: arm_scmi: Create debugfs files for statistics

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ZovysXW92kgiKlet@pluto>
Date: Mon, 8 Jul 2024 15:07:45 +0100
From: Cristian Marussi <cristian.marussi@....com>
To: Luke Parkin <luke.parkin@....com>
Cc: linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	arm-scmi@...r.kernel.org, sudeep.holla@....com,
	cristian.marussi@....com
Subject: Re: [PATCH v2 4/4] firmware: arm_scmi: Create debugfs files for
 statistics

On Wed, Jul 03, 2024 at 03:37:38PM +0100, Luke Parkin wrote:
> Create debugfs files for the statistics in the scmi_debug_stats struct
> 
> Signed-off-by: Luke Parkin <luke.parkin@....com>

Missing ---

> v1->v2
> Only create stats pointer if stats are enabled
> Move stats debugfs creation into a seperate helper function
> ---
>  drivers/firmware/arm_scmi/driver.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
> index 937546397cf2..10cd9a319ffb 100644
> --- a/drivers/firmware/arm_scmi/driver.c
> +++ b/drivers/firmware/arm_scmi/driver.c
> @@ -2858,6 +2858,24 @@ static int scmi_device_request_notifier(struct notifier_block *nb,
>  	return NOTIFY_OK;
>  }
>  
> +static void scmi_debugfs_stats_setup(struct scmi_info *info,
> +				     struct dentry *trans)
> +{
> +	struct dentry *stats;
> +
> +	stats = debugfs_create_dir("stats", trans);
> +	debugfs_create_atomic_t("response_ok", 0400, stats,
> +				&info->stats.response_ok);
> +	debugfs_create_atomic_t("dlyd_response_ok", 0400, stats,
> +				&info->stats.dlyd_response_ok);
> +	debugfs_create_atomic_t("sent_ok", 0400, stats,
> +				&info->stats.sent_ok);
> +	debugfs_create_atomic_t("sent_fail", 0400, stats,
> +				&info->stats.sent_fail);
> +	debugfs_create_atomic_t("xfers_response_timeout", 0400, stats,
> +				&info->stats.xfers_response_timeout);
> +}
> +
>  static void scmi_debugfs_common_cleanup(void *d)
>  {
>  	struct scmi_debug_info *dbg = d;
> @@ -2924,6 +2942,9 @@ static struct scmi_debug_info *scmi_debugfs_common_setup(struct scmi_info *info)
>  	debugfs_create_u32("rx_max_msg", 0400, trans,
>  			   (u32 *)&info->rx_minfo.max_msg);
>  
> +	if (IS_ENABLED(CONFIG_ARM_SCMI_DEBUG_STATISTICS))
> +		scmi_debugfs_stats_setup(info, trans);
> +

Nothing to say here if not that more entries will need to be added as said.

Moreover, You could take the chance in V3 to add in this patch the
support to handle resetting each single counter (hint...this is a low hanging
fruit :D) and also to support something like:

	transports/stats/reset

which will be a WOnly booolean entry that will reset ALL the counters in
one go.

Thanks,
Cristian