| Message ID | ca32bed9956a00d1617ae801ca440ed02fe39570.1712788087.git.ben.hutchings@mind.be |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | mtd: ubi: Fix some initialisation failure cases | expand |
在 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; > }
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; - } 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; - } #ifdef CONFIG_MTD_UBI_FAULT_INJECTION dfs_create_fault_entry(dfs_rootdir); #endif - - return 0; } /** @@ -585,10 +568,9 @@ static const struct file_operations eraseblk_count_fops = { * ubi_debugfs_init_dev - initialize debugfs for an UBI device. * @ubi: UBI device description object * - * This function creates all debugfs files for UBI device @ubi. Returns zero in - * case of success and a negative error code in case of failure. + * This function tries to create all debugfs files for UBI device @ubi. */ -int ubi_debugfs_init_dev(struct ubi_device *ubi) +void ubi_debugfs_init_dev(struct ubi_device *ubi) { unsigned long ubi_num = ubi->ubi_num; struct ubi_debug_info *d = &ubi->dbg; @@ -596,13 +578,13 @@ int ubi_debugfs_init_dev(struct ubi_device *ubi) int n; if (!IS_ENABLED(CONFIG_DEBUG_FS)) - return 0; + return; n = snprintf(d->dfs_dir_name, UBI_DFS_DIR_LEN + 1, UBI_DFS_DIR_NAME, ubi->ubi_num); if (n > UBI_DFS_DIR_LEN) { /* The array size is too small */ - return -EINVAL; + return; } d->dfs_dir = debugfs_create_dir(d->dfs_dir_name, dfs_rootdir); @@ -653,7 +635,6 @@ int ubi_debugfs_init_dev(struct ubi_device *ubi) (void *)ubi_num, &dfs_fops); #endif - return 0; } /** diff --git a/drivers/mtd/ubi/debug.h b/drivers/mtd/ubi/debug.h index b2fd97548808..1dc430d3f482 100644 --- a/drivers/mtd/ubi/debug.h +++ b/drivers/mtd/ubi/debug.h @@ -47,9 +47,9 @@ void ubi_dump_aeb(const struct ubi_ainf_peb *aeb, int type); void ubi_dump_mkvol_req(const struct ubi_mkvol_req *req); int ubi_self_check_all_ff(struct ubi_device *ubi, int pnum, int offset, int len); -int ubi_debugfs_init(void); +void ubi_debugfs_init(void); void ubi_debugfs_exit(void); -int ubi_debugfs_init_dev(struct ubi_device *ubi); +void ubi_debugfs_init_dev(struct ubi_device *ubi); void ubi_debugfs_exit_dev(struct ubi_device *ubi); /**
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(-)