Re: [PATCH 1/4] bcache: remove the cache_dev_name field from struct cache

[Coly Li <colyli@xxxxxxx>]

On 10/18/21 2:09 PM, Christoph Hellwig wrote:
> Just use the %pg format specifier to print the name directly.

Hi  Christoph,

NACK for this patch.  The buffer cache_dev_name is added on purpose, in 
case ca->bdev cannot be referenced correctly for some special condition 
when underlying device fails.

Thanks.

Coly Li
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
>   drivers/md/bcache/bcache.h | 2 --
>   drivers/md/bcache/io.c     | 8 ++++----
>   drivers/md/bcache/super.c  | 7 +++----
>   3 files changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/md/bcache/bcache.h b/drivers/md/bcache/bcache.h
> index 5fc989a6d4528..47ff9ecea2e29 100644
> --- a/drivers/md/bcache/bcache.h
> +++ b/drivers/md/bcache/bcache.h
> @@ -470,8 +470,6 @@ struct cache {
>   	atomic_long_t		meta_sectors_written;
>   	atomic_long_t		btree_sectors_written;
>   	atomic_long_t		sectors_written;
> -
> -	char			cache_dev_name[BDEVNAME_SIZE];
>   };
>   
>   struct gc_stat {
> diff --git a/drivers/md/bcache/io.c b/drivers/md/bcache/io.c
> index e4388fe3ab7ef..564357de76404 100644
> --- a/drivers/md/bcache/io.c
> +++ b/drivers/md/bcache/io.c
> @@ -123,13 +123,13 @@ void bch_count_io_errors(struct cache *ca,
>   		errors >>= IO_ERROR_SHIFT;
>   
>   		if (errors < ca->set->error_limit)
> -			pr_err("%s: IO error on %s%s\n",
> -			       ca->cache_dev_name, m,
> +			pr_err("%pg: IO error on %s%s\n",
> +			       ca->bdev, m,
>   			       is_read ? ", recovering." : ".");
>   		else
>   			bch_cache_set_error(ca->set,
> -					    "%s: too many IO errors %s\n",
> -					    ca->cache_dev_name, m);
> +					    "%pg: too many IO errors %s\n",
> +					    ca->bdev, m);
>   	}
>   }
>   
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index f2874c77ff797..d0d0257252adc 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -2338,7 +2338,7 @@ static int cache_alloc(struct cache *ca)
>   err_free:
>   	module_put(THIS_MODULE);
>   	if (err)
> -		pr_notice("error %s: %s\n", ca->cache_dev_name, err);
> +		pr_notice("error %pg: %s\n", ca->bdev, err);
>   	return ret;
>   }
>   
> @@ -2348,7 +2348,6 @@ static int register_cache(struct cache_sb *sb, struct cache_sb_disk *sb_disk,
>   	const char *err = NULL; /* must be set for any error case */
>   	int ret = 0;
>   
> -	bdevname(bdev, ca->cache_dev_name);
>   	memcpy(&ca->sb, sb, sizeof(struct cache_sb));
>   	ca->bdev = bdev;
>   	ca->bdev->bd_holder = ca;
> @@ -2390,14 +2389,14 @@ static int register_cache(struct cache_sb *sb, struct cache_sb_disk *sb_disk,
>   		goto out;
>   	}
>   
> -	pr_info("registered cache device %s\n", ca->cache_dev_name);
> +	pr_info("registered cache device %pg\n", ca->bdev);
>   
>   out:
>   	kobject_put(&ca->kobj);
>   
>   err:
>   	if (err)
> -		pr_notice("error %s: %s\n", ca->cache_dev_name, err);
> +		pr_notice("error %pg: %s\n", ca->bdev, err);
>   
>   	return ret;
>   }