Re: [PATCH 1/1] RAS/AMD/FMPM: Fix return value check in setup_debugfs()

[Yazen Ghannam <yazen.ghannam@amd.com>]

Hi Zhen Lei,

Thanks for the patch.

The subject line does not need to include 1/1 for a single patch.

On Wed, Oct 23, 2024 at 03:05:08PM +0800, Zhen Lei wrote:
> Fix the incorrect return value check for debugfs_create_dir() and
> debugfs_create_file(), which returns ERR_PTR(-ERROR) instead of NULL

Yes, the return value check is incorrect. But I think the solution is
simpler. Please see the patch at the end.

> when it fails. In addition, fmpm_dfs_dir should be set to NULL after
> being reclaimed.
>

Why is this needed?

> Fixes: 7d19eea51757 ("RAS/AMD/FMPM: Add debugfs interface to print record entries")
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
>  drivers/ras/amd/fmpm.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ras/amd/fmpm.c b/drivers/ras/amd/fmpm.c
> index 90de737fbc90978..b1cffbde6d319ed 100644
> --- a/drivers/ras/amd/fmpm.c
> +++ b/drivers/ras/amd/fmpm.c
> @@ -956,12 +956,14 @@ static void setup_debugfs(void)
>  		return;
>  
>  	fmpm_dfs_dir = debugfs_create_dir("fmpm", dfs);
> -	if (!fmpm_dfs_dir)
> +	if (IS_ERR(fmpm_dfs_dir))
>  		return;
>  
>  	fmpm_dfs_entries = debugfs_create_file("entries", 0400, fmpm_dfs_dir, NULL, &fmpm_fops);
> -	if (!fmpm_dfs_entries)
> +	if (IS_ERR(fmpm_dfs_entries)) {
>  		debugfs_remove(fmpm_dfs_dir);
> +		fmpm_dfs_dir = NULL;
> +	}
>  }
>

I think the intention here is correct. But the solution is to just
remove the error checks entirely.

Thanks,
Yazen


From 75869583bec8eb95d062cecedc4278bead8a293a Mon Sep 17 00:00:00 2001
From: Yazen Ghannam <yazen.ghannam@amd.com>
Date: Thu, 24 Oct 2024 10:06:54 -0400
Subject: [PATCH] RAS/AMD/ATL: Simplify debugfs init

Drop return value checks as debugfs API says most drivers should ignore
errors. Issues between creation steps are automatically handled by the
debugfs code.

Also, add a code comment highlighting why a valid ras debugfs dentry is
needed.

Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
---
 drivers/ras/amd/fmpm.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/ras/amd/fmpm.c b/drivers/ras/amd/fmpm.c
index 90de737fbc90..3965963d9164 100644
--- a/drivers/ras/amd/fmpm.c
+++ b/drivers/ras/amd/fmpm.c
@@ -952,16 +952,15 @@ static void setup_debugfs(void)
 {
 	struct dentry *dfs = ras_get_debugfs_root();
 
+	/*
+	 * Ensure there's a ras debugfs directory so the fmpm files aren't populated in
+	 * the root debugfs directory.
+	 */
 	if (!dfs)
 		return;
 
 	fmpm_dfs_dir = debugfs_create_dir("fmpm", dfs);
-	if (!fmpm_dfs_dir)
-		return;
-
 	fmpm_dfs_entries = debugfs_create_file("entries", 0400, fmpm_dfs_dir, NULL, &fmpm_fops);
-	if (!fmpm_dfs_entries)
-		debugfs_remove(fmpm_dfs_dir);
 }
 
 static const struct x86_cpu_id fmpm_cpuids[] = {
-- 
2.43.0