Re: [PATCH 2/2] mtd: ubi: Ignore all debugfs initialisation failures

[Zhihao Cheng <chengzhihao1@huawei.com>]

在 2024/4/11 6:42, Ben Hutchings 写道:
> Drivers should never stop initialising or probing because of a failure
> to create debugfs entries.  Such failures are harmless and could be
> the intended result of setting the kernel parameter debugfs=off.
> 
> This was partially fixed in 2019, but some error checks were missed
> and another one was been added since.
> 
> Signed-off-by: Ben Hutchings <ben.hutchings@mind.be>
> Fixes: 2a734bb8d502 ("UBI: use debugfs for the extra checks knobs")
> Fixes: c2d73ba892ea ("mtd: no need to check return value of ...")
> Fixes: 6931fb44858c ("ubi: Use the fault injection framework to enhance ...")
> ---
>   drivers/mtd/ubi/build.c | 10 ++--------
>   drivers/mtd/ubi/debug.c | 33 +++++++--------------------------
>   drivers/mtd/ubi/debug.h |  4 ++--
>   3 files changed, 11 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
> index 50d78975b3ed..c07a4754b521 100644
> --- a/drivers/mtd/ubi/build.c
> +++ b/drivers/mtd/ubi/build.c
> @@ -1018,9 +1018,7 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num,
>   	if (err)
>   		goto out_detach;
>   
> -	err = ubi_debugfs_init_dev(ubi);
> -	if (err)
> -		goto out_uif;
> +	ubi_debugfs_init_dev(ubi);
>   
>   	ubi->bgt_thread = kthread_create(ubi_thread, ubi, "%s", ubi->bgt_name);
>   	if (IS_ERR(ubi->bgt_thread)) {
> @@ -1064,7 +1062,6 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num,
>   
>   out_debugfs:
>   	ubi_debugfs_exit_dev(ubi);
> -out_uif:
>   	uif_close(ubi);
>   out_detach:
>   	ubi_wl_close(ubi);
> @@ -1362,9 +1359,7 @@ static int __init ubi_init(void)
>   		goto out_dev_unreg;
>   	}
>   
> -	err = ubi_debugfs_init();
> -	if (err)
> -		goto out_slab;
> +	ubi_debugfs_init();
>   
>   	err = ubiblock_init();
>   	if (err) {
> @@ -1390,7 +1385,6 @@ static int __init ubi_init(void)
>   	ubiblock_exit();
>   out_debugfs:
>   	ubi_debugfs_exit();
> -out_slab:
>   	kmem_cache_destroy(ubi_wl_entry_slab);
>   out_dev_unreg:
>   	misc_deregister(&ubi_ctrl_cdev);
> diff --git a/drivers/mtd/ubi/debug.c b/drivers/mtd/ubi/debug.c
> index d57f52bd2ff3..505bc07d7dbb 100644
> --- a/drivers/mtd/ubi/debug.c
> +++ b/drivers/mtd/ubi/debug.c
> @@ -248,13 +248,6 @@ static void dfs_create_fault_entry(struct dentry *parent)
>   	struct dentry *dir;
>   
>   	dir = debugfs_create_dir("fault_inject", parent);
> -	if (IS_ERR_OR_NULL(dir)) {
> -		int err = dir ? PTR_ERR(dir) : -ENODEV;
> -
> -		pr_warn("UBI error: cannot create \"fault_inject\" debugfs directory, error %d\n",
> -			 err);
> -		return;
> -	}

I think we can keep this error path, sub dirs won't be created by 
following functions if dir is assigned as error code. so ubi could 
return in advance and prints a warning message to notify user that fault 
injection debugfs initialization is failed.
>   
>   	fault_create_debugfs_attr("emulate_eccerr", dir,
>   				  &fault_eccerr_attr);
> @@ -291,28 +284,18 @@ static void dfs_create_fault_entry(struct dentry *parent)
>   /**
>    * ubi_debugfs_init - create UBI debugfs directory.
>    *
> - * Create UBI debugfs directory. Returns zero in case of success and a negative
> - * error code in case of failure.
> + * Try to create UBI debugfs directory.
>    */
> -int ubi_debugfs_init(void)
> +void ubi_debugfs_init(void)
>   {
>   	if (!IS_ENABLED(CONFIG_DEBUG_FS))
> -		return 0;
> +		return;
>   
>   	dfs_rootdir = debugfs_create_dir("ubi", NULL);
> -	if (IS_ERR_OR_NULL(dfs_rootdir)) {
> -		int err = dfs_rootdir ? PTR_ERR(dfs_rootdir) : -ENODEV;
> -
> -		pr_err("UBI error: cannot create \"ubi\" debugfs directory, error %d\n",
> -		       err);
> -		return err;
> -	}

Similar to previous, also replace 'pr_err' with 'pr_warn'.
>   
>   #ifdef CONFIG_MTD_UBI_FAULT_INJECTION
>   	dfs_create_fault_entry(dfs_rootdir);
>   #endif
> -
> -	return 0;
>   }

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/