Re: [PATCH] bcache: do not check return value of debugfs_create_dir()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Coly!

2018-05-22 9:10 GMT+02:00 Coly Li <colyli@xxxxxxx>:
> Greg KH suggests that normal code should not care about debugfs. Therefore
> no matter successful or failed of debugfs_create_dir() execution, it is
> unncessary to check its return value.
>
> There are two functions called debugfs_create_dir() and check the return
> value, which are bch_debug_init() and closure_debug_init(). This patch
> changes these two functions from int to void type, and ignore return values
> of debugfs_create_dir().
>
> This patch does not fix exact bug, just makes things work as they should.

I applied the patch to master and cherry-picked to my 4.16 branch
(cleaning up the conflicts).

I'm not sure if I did the conflict in closure_debug right but as I
compiled with CONFIG_DEBUG_FS=n, I think it doesn't matter for my
configuration.

The resulting patch works, currently running it while writing this message.

So you can add my tested-by if it makes sense.

Regards,
Kai


> Signed-off-by: Coly Li <colyli@xxxxxxx>
> Suggested-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Cc: Kai Krakow <kai@xxxxxxxxxxx>
> Cc: Kent Overstreet <kent.overstreet@xxxxxxxxx>
> ---
>  drivers/md/bcache/bcache.h  |  2 +-
>  drivers/md/bcache/closure.c | 13 +++++++++----
>  drivers/md/bcache/closure.h |  4 ++--
>  drivers/md/bcache/debug.c   | 11 ++++++-----
>  drivers/md/bcache/super.c   |  4 +++-
>  5 files changed, 21 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/md/bcache/bcache.h b/drivers/md/bcache/bcache.h
> index 3a0cfb237af9..bee6251d2d40 100644
> --- a/drivers/md/bcache/bcache.h
> +++ b/drivers/md/bcache/bcache.h
> @@ -995,7 +995,7 @@ void bch_open_buckets_free(struct cache_set *);
>  int bch_cache_allocator_start(struct cache *ca);
>
>  void bch_debug_exit(void);
> -int bch_debug_init(struct kobject *);
> +void bch_debug_init(struct kobject *);
>  void bch_request_exit(void);
>  int bch_request_init(void);
>
> diff --git a/drivers/md/bcache/closure.c b/drivers/md/bcache/closure.c
> index 0e14969182c6..618253683d40 100644
> --- a/drivers/md/bcache/closure.c
> +++ b/drivers/md/bcache/closure.c
> @@ -199,11 +199,16 @@ static const struct file_operations debug_ops = {
>         .release        = single_release
>  };
>
> -int __init closure_debug_init(void)
> +void  __init closure_debug_init(void)
>  {
> -       closure_debug = debugfs_create_file("closures",
> -                               0400, bcache_debug, NULL, &debug_ops);
> -       return IS_ERR_OR_NULL(closure_debug);
> +       if (!IS_ERR_OR_NULL(bcache_debug))
> +               /*
> +                * it is unnecessary to check return value of
> +                * debugfs_create_file(), we should not care
> +                * about this.
> +                */
> +               closure_debug = debugfs_create_file(
> +                       "closures", 0400, bcache_debug, NULL, &debug_ops);
>  }
>  #endif
>
> diff --git a/drivers/md/bcache/closure.h b/drivers/md/bcache/closure.h
> index 71427eb5fdae..7c2c5bc7c88b 100644
> --- a/drivers/md/bcache/closure.h
> +++ b/drivers/md/bcache/closure.h
> @@ -186,13 +186,13 @@ static inline void closure_sync(struct closure *cl)
>
>  #ifdef CONFIG_BCACHE_CLOSURES_DEBUG
>
> -int closure_debug_init(void);
> +void closure_debug_init(void);
>  void closure_debug_create(struct closure *cl);
>  void closure_debug_destroy(struct closure *cl);
>
>  #else
>
> -static inline int closure_debug_init(void) { return 0; }
> +static inline void closure_debug_init(void) {}
>  static inline void closure_debug_create(struct closure *cl) {}
>  static inline void closure_debug_destroy(struct closure *cl) {}
>
> diff --git a/drivers/md/bcache/debug.c b/drivers/md/bcache/debug.c
> index d030ce3025a6..57f8f5aeee55 100644
> --- a/drivers/md/bcache/debug.c
> +++ b/drivers/md/bcache/debug.c
> @@ -248,11 +248,12 @@ void bch_debug_exit(void)
>                 debugfs_remove_recursive(bcache_debug);
>  }
>
> -int __init bch_debug_init(struct kobject *kobj)
> +void __init bch_debug_init(struct kobject *kobj)
>  {
> -       if (!IS_ENABLED(CONFIG_DEBUG_FS))
> -               return 0;
> -
> +       /*
> +        * it is unnecessary to check return value of
> +        * debugfs_create_file(), we should not care
> +        * about this.
> +        */
>         bcache_debug = debugfs_create_dir("bcache", NULL);
> -       return IS_ERR_OR_NULL(bcache_debug);
>  }
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index 3dea06b41d43..2b62671e4d83 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -2301,10 +2301,12 @@ static int __init bcache_init(void)
>         if (!(bcache_wq = alloc_workqueue("bcache", WQ_MEM_RECLAIM, 0)) ||
>             !(bcache_kobj = kobject_create_and_add("bcache", fs_kobj)) ||
>             bch_request_init() ||
> -           bch_debug_init(bcache_kobj) || closure_debug_init() ||
>             sysfs_create_files(bcache_kobj, files))
>                 goto err;
>
> +       bch_debug_init(bcache_kobj);
> +       closure_debug_init();
> +
>         return 0;
>  err:
>         bcache_exit();
> --
> 2.16.3
>



[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux