[PATCH v2 0/2] bcache: Fix block device claiming

[PATCH v2 0/2] bcache: Fix block device claiming

[Jan Kara]

Hello,

these two patches fix block device claiming for bcache broken by recent
Christoph's changes to blkdev_get_*() functions and also cleans up the layering
violation inside bcache which was the underlying cause of the breakage.

This time I've actually tested various modified error handling paths (which was
good because one of them was indeed wrong!).

Jens, please consider merging these fixes before sending Linus a pull request
with blkdev changes.

Changes since v1:
* Fix compile breakage spotted by 0-day
* Fix error handling path when the second blkdev_get_by_dev() fails
* Fix commit message of patch 2/2

								Honza
Previous versions:
Link: http://lore.kernel.org/r/20230621162024.29310-1-jack@suse.cz # v1

[PATCH v2 1/2] bcache: Alloc holder object before async registration

[Jan Kara]

Allocate holder object (cache or cached_dev) before offloading the
rest of the startup to async work. This will allow us to open the block
block device with proper holder.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 drivers/md/bcache/super.c | 66 +++++++++++++++------------------------
 1 file changed, 25 insertions(+), 41 deletions(-)

diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index e2a803683105..913dd94353b6 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -2448,6 +2448,7 @@ struct async_reg_args {
 	struct cache_sb *sb;
 	struct cache_sb_disk *sb_disk;
 	struct block_device *bdev;
+	void *holder;
 };
 
 static void register_bdev_worker(struct work_struct *work)
@@ -2455,22 +2456,13 @@ static void register_bdev_worker(struct work_struct *work)
 	int fail = false;
 	struct async_reg_args *args =
 		container_of(work, struct async_reg_args, reg_work.work);
-	struct cached_dev *dc;
-
-	dc = kzalloc(sizeof(*dc), GFP_KERNEL);
-	if (!dc) {
-		fail = true;
-		put_page(virt_to_page(args->sb_disk));
-		blkdev_put(args->bdev, bcache_kobj);
-		goto out;
-	}
 
 	mutex_lock(&bch_register_lock);
-	if (register_bdev(args->sb, args->sb_disk, args->bdev, dc) < 0)
+	if (register_bdev(args->sb, args->sb_disk, args->bdev, args->holder)
+	    < 0)
 		fail = true;
 	mutex_unlock(&bch_register_lock);
 
-out:
 	if (fail)
 		pr_info("error %s: fail to register backing device\n",
 			args->path);
@@ -2485,21 +2477,11 @@ static void register_cache_worker(struct work_struct *work)
 	int fail = false;
 	struct async_reg_args *args =
 		container_of(work, struct async_reg_args, reg_work.work);
-	struct cache *ca;
-
-	ca = kzalloc(sizeof(*ca), GFP_KERNEL);
-	if (!ca) {
-		fail = true;
-		put_page(virt_to_page(args->sb_disk));
-		blkdev_put(args->bdev, bcache_kobj);
-		goto out;
-	}
 
 	/* blkdev_put() will be called in bch_cache_release() */
-	if (register_cache(args->sb, args->sb_disk, args->bdev, ca) != 0)
+	if (register_cache(args->sb, args->sb_disk, args->bdev, args->holder))
 		fail = true;
 
-out:
 	if (fail)
 		pr_info("error %s: fail to register cache device\n",
 			args->path);
@@ -2520,6 +2502,13 @@ static void register_device_async(struct async_reg_args *args)
 	queue_delayed_work(system_wq, &args->reg_work, 10);
 }
 
+static void *alloc_holder_object(struct cache_sb *sb)
+{
+	if (SB_IS_BDEV(sb))
+		return kzalloc(sizeof(struct cached_dev), GFP_KERNEL);
+	return kzalloc(sizeof(struct cache), GFP_KERNEL);
+}
+
 static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
 			       const char *buffer, size_t size)
 {
@@ -2528,6 +2517,7 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
 	struct cache_sb *sb;
 	struct cache_sb_disk *sb_disk;
 	struct block_device *bdev;
+	void *holder;
 	ssize_t ret;
 	bool async_registration = false;
 
@@ -2585,6 +2575,13 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
 	if (err)
 		goto out_blkdev_put;
 
+	holder = alloc_holder_object(sb);
+	if (!holder) {
+		ret = -ENOMEM;
+		err = "cannot allocate memory";
+		goto out_put_sb_page;
+	}
+
 	err = "failed to register device";
 
 	if (async_registration) {
@@ -2595,44 +2592,29 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
 		if (!args) {
 			ret = -ENOMEM;
 			err = "cannot allocate memory";
-			goto out_put_sb_page;
+			goto out_free_holder;
 		}
 
 		args->path	= path;
 		args->sb	= sb;
 		args->sb_disk	= sb_disk;
 		args->bdev	= bdev;
+		args->holder	= holder;
 		register_device_async(args);
 		/* No wait and returns to user space */
 		goto async_done;
 	}
 
 	if (SB_IS_BDEV(sb)) {
-		struct cached_dev *dc = kzalloc(sizeof(*dc), GFP_KERNEL);
-
-		if (!dc) {
-			ret = -ENOMEM;
-			err = "cannot allocate memory";
-			goto out_put_sb_page;
-		}
-
 		mutex_lock(&bch_register_lock);
-		ret = register_bdev(sb, sb_disk, bdev, dc);
+		ret = register_bdev(sb, sb_disk, bdev, holder);
 		mutex_unlock(&bch_register_lock);
 		/* blkdev_put() will be called in cached_dev_free() */
 		if (ret < 0)
 			goto out_free_sb;
 	} else {
-		struct cache *ca = kzalloc(sizeof(*ca), GFP_KERNEL);
-
-		if (!ca) {
-			ret = -ENOMEM;
-			err = "cannot allocate memory";
-			goto out_put_sb_page;
-		}
-
 		/* blkdev_put() will be called in bch_cache_release() */
-		ret = register_cache(sb, sb_disk, bdev, ca);
+		ret = register_cache(sb, sb_disk, bdev, holder);
 		if (ret)
 			goto out_free_sb;
 	}
@@ -2644,6 +2626,8 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
 async_done:
 	return size;
 
+out_free_holder:
+	kfree(holder);
 out_put_sb_page:
 	put_page(virt_to_page(sb_disk));
 out_blkdev_put:
-- 
2.35.3

Re: [PATCH v2 1/2] bcache: Alloc holder object before async registration

[Kent Overstreet]

On Thu, Jun 22, 2023 at 06:46:54PM +0200, Jan Kara wrote:
> Allocate holder object (cache or cached_dev) before offloading the
> rest of the startup to async work. This will allow us to open the block
> block device with proper holder.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>

Reviewed-by: Kent Overstreet <kent.overstreet@linux.dev>

> ---
>  drivers/md/bcache/super.c | 66 +++++++++++++++------------------------
>  1 file changed, 25 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index e2a803683105..913dd94353b6 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -2448,6 +2448,7 @@ struct async_reg_args {
>  	struct cache_sb *sb;
>  	struct cache_sb_disk *sb_disk;
>  	struct block_device *bdev;
> +	void *holder;
>  };
>  
>  static void register_bdev_worker(struct work_struct *work)
> @@ -2455,22 +2456,13 @@ static void register_bdev_worker(struct work_struct *work)
>  	int fail = false;
>  	struct async_reg_args *args =
>  		container_of(work, struct async_reg_args, reg_work.work);
> -	struct cached_dev *dc;
> -
> -	dc = kzalloc(sizeof(*dc), GFP_KERNEL);
> -	if (!dc) {
> -		fail = true;
> -		put_page(virt_to_page(args->sb_disk));
> -		blkdev_put(args->bdev, bcache_kobj);
> -		goto out;
> -	}
>  
>  	mutex_lock(&bch_register_lock);
> -	if (register_bdev(args->sb, args->sb_disk, args->bdev, dc) < 0)
> +	if (register_bdev(args->sb, args->sb_disk, args->bdev, args->holder)
> +	    < 0)
>  		fail = true;
>  	mutex_unlock(&bch_register_lock);
>  
> -out:
>  	if (fail)
>  		pr_info("error %s: fail to register backing device\n",
>  			args->path);
> @@ -2485,21 +2477,11 @@ static void register_cache_worker(struct work_struct *work)
>  	int fail = false;
>  	struct async_reg_args *args =
>  		container_of(work, struct async_reg_args, reg_work.work);
> -	struct cache *ca;
> -
> -	ca = kzalloc(sizeof(*ca), GFP_KERNEL);
> -	if (!ca) {
> -		fail = true;
> -		put_page(virt_to_page(args->sb_disk));
> -		blkdev_put(args->bdev, bcache_kobj);
> -		goto out;
> -	}
>  
>  	/* blkdev_put() will be called in bch_cache_release() */
> -	if (register_cache(args->sb, args->sb_disk, args->bdev, ca) != 0)
> +	if (register_cache(args->sb, args->sb_disk, args->bdev, args->holder))
>  		fail = true;
>  
> -out:
>  	if (fail)
>  		pr_info("error %s: fail to register cache device\n",
>  			args->path);
> @@ -2520,6 +2502,13 @@ static void register_device_async(struct async_reg_args *args)
>  	queue_delayed_work(system_wq, &args->reg_work, 10);
>  }
>  
> +static void *alloc_holder_object(struct cache_sb *sb)
> +{
> +	if (SB_IS_BDEV(sb))
> +		return kzalloc(sizeof(struct cached_dev), GFP_KERNEL);
> +	return kzalloc(sizeof(struct cache), GFP_KERNEL);
> +}
> +
>  static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
>  			       const char *buffer, size_t size)
>  {
> @@ -2528,6 +2517,7 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
>  	struct cache_sb *sb;
>  	struct cache_sb_disk *sb_disk;
>  	struct block_device *bdev;
> +	void *holder;
>  	ssize_t ret;
>  	bool async_registration = false;
>  
> @@ -2585,6 +2575,13 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
>  	if (err)
>  		goto out_blkdev_put;
>  
> +	holder = alloc_holder_object(sb);
> +	if (!holder) {
> +		ret = -ENOMEM;
> +		err = "cannot allocate memory";
> +		goto out_put_sb_page;
> +	}
> +
>  	err = "failed to register device";
>  
>  	if (async_registration) {
> @@ -2595,44 +2592,29 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
>  		if (!args) {
>  			ret = -ENOMEM;
>  			err = "cannot allocate memory";
> -			goto out_put_sb_page;
> +			goto out_free_holder;
>  		}
>  
>  		args->path	= path;
>  		args->sb	= sb;
>  		args->sb_disk	= sb_disk;
>  		args->bdev	= bdev;
> +		args->holder	= holder;
>  		register_device_async(args);
>  		/* No wait and returns to user space */
>  		goto async_done;
>  	}
>  
>  	if (SB_IS_BDEV(sb)) {
> -		struct cached_dev *dc = kzalloc(sizeof(*dc), GFP_KERNEL);
> -
> -		if (!dc) {
> -			ret = -ENOMEM;
> -			err = "cannot allocate memory";
> -			goto out_put_sb_page;
> -		}
> -
>  		mutex_lock(&bch_register_lock);
> -		ret = register_bdev(sb, sb_disk, bdev, dc);
> +		ret = register_bdev(sb, sb_disk, bdev, holder);
>  		mutex_unlock(&bch_register_lock);
>  		/* blkdev_put() will be called in cached_dev_free() */
>  		if (ret < 0)
>  			goto out_free_sb;
>  	} else {
> -		struct cache *ca = kzalloc(sizeof(*ca), GFP_KERNEL);
> -
> -		if (!ca) {
> -			ret = -ENOMEM;
> -			err = "cannot allocate memory";
> -			goto out_put_sb_page;
> -		}
> -
>  		/* blkdev_put() will be called in bch_cache_release() */
> -		ret = register_cache(sb, sb_disk, bdev, ca);
> +		ret = register_cache(sb, sb_disk, bdev, holder);
>  		if (ret)
>  			goto out_free_sb;
>  	}
> @@ -2644,6 +2626,8 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
>  async_done:
>  	return size;
>  
> +out_free_holder:
> +	kfree(holder);
>  out_put_sb_page:
>  	put_page(virt_to_page(sb_disk));
>  out_blkdev_put:
> -- 
> 2.35.3
> 

Re: [PATCH v2 1/2] bcache: Alloc holder object before async registration

[Coly Li]

On Thu, Jun 22, 2023 at 06:46:54PM +0200, Jan Kara wrote:
> Allocate holder object (cache or cached_dev) before offloading the
> rest of the startup to async work. This will allow us to open the block
> block device with proper holder.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>

Acked-by: Coly Li <colyli@suse.de>

Thanks.

Coly Li

> ---
> drivers/md/bcache/super.c | 66 +++++++++++++++------------------------
> 1 file changed, 25 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index e2a803683105..913dd94353b6 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -2448,6 +2448,7 @@ struct async_reg_args {
> 	struct cache_sb *sb;
> 	struct cache_sb_disk *sb_disk;
> 	struct block_device *bdev;
> +	void *holder;
> };
> 
> static void register_bdev_worker(struct work_struct *work)
> @@ -2455,22 +2456,13 @@ static void register_bdev_worker(struct work_struct *work)
> 	int fail = false;
> 	struct async_reg_args *args =
> 		container_of(work, struct async_reg_args, reg_work.work);
> -	struct cached_dev *dc;
> -
> -	dc = kzalloc(sizeof(*dc), GFP_KERNEL);
> -	if (!dc) {
> -		fail = true;
> -		put_page(virt_to_page(args->sb_disk));
> -		blkdev_put(args->bdev, bcache_kobj);
> -		goto out;
> -	}
> 
> 	mutex_lock(&bch_register_lock);
> -	if (register_bdev(args->sb, args->sb_disk, args->bdev, dc) < 0)
> +	if (register_bdev(args->sb, args->sb_disk, args->bdev, args->holder)
> +	    < 0)
> 		fail = true;
> 	mutex_unlock(&bch_register_lock);
> 
> -out:
> 	if (fail)
> 		pr_info("error %s: fail to register backing device\n",
> 			args->path);
> @@ -2485,21 +2477,11 @@ static void register_cache_worker(struct work_struct *work)
> 	int fail = false;
> 	struct async_reg_args *args =
> 		container_of(work, struct async_reg_args, reg_work.work);
> -	struct cache *ca;
> -
> -	ca = kzalloc(sizeof(*ca), GFP_KERNEL);
> -	if (!ca) {
> -		fail = true;
> -		put_page(virt_to_page(args->sb_disk));
> -		blkdev_put(args->bdev, bcache_kobj);
> -		goto out;
> -	}
> 
> 	/* blkdev_put() will be called in bch_cache_release() */
> -	if (register_cache(args->sb, args->sb_disk, args->bdev, ca) != 0)
> +	if (register_cache(args->sb, args->sb_disk, args->bdev, args->holder))
> 		fail = true;
> 
> -out:
> 	if (fail)
> 		pr_info("error %s: fail to register cache device\n",
> 			args->path);
> @@ -2520,6 +2502,13 @@ static void register_device_async(struct async_reg_args *args)
> 	queue_delayed_work(system_wq, &args->reg_work, 10);
> }
> 
> +static void *alloc_holder_object(struct cache_sb *sb)
> +{
> +	if (SB_IS_BDEV(sb))
> +		return kzalloc(sizeof(struct cached_dev), GFP_KERNEL);
> +	return kzalloc(sizeof(struct cache), GFP_KERNEL);
> +}
> +
> static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
> 			       const char *buffer, size_t size)
> {
> @@ -2528,6 +2517,7 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
> 	struct cache_sb *sb;
> 	struct cache_sb_disk *sb_disk;
> 	struct block_device *bdev;
> +	void *holder;
> 	ssize_t ret;
> 	bool async_registration = false;
> 
> @@ -2585,6 +2575,13 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
> 	if (err)
> 		goto out_blkdev_put;
> 
> +	holder = alloc_holder_object(sb);
> +	if (!holder) {
> +		ret = -ENOMEM;
> +		err = "cannot allocate memory";
> +		goto out_put_sb_page;
> +	}
> +
> 	err = "failed to register device";
> 
> 	if (async_registration) {
> @@ -2595,44 +2592,29 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
> 		if (!args) {
> 			ret = -ENOMEM;
> 			err = "cannot allocate memory";
> -			goto out_put_sb_page;
> +			goto out_free_holder;
> 		}
> 
> 		args->path	= path;
> 		args->sb	= sb;
> 		args->sb_disk	= sb_disk;
> 		args->bdev	= bdev;
> +		args->holder	= holder;
> 		register_device_async(args);
> 		/* No wait and returns to user space */
> 		goto async_done;
> 	}
> 
> 	if (SB_IS_BDEV(sb)) {
> -		struct cached_dev *dc = kzalloc(sizeof(*dc), GFP_KERNEL);
> -
> -		if (!dc) {
> -			ret = -ENOMEM;
> -			err = "cannot allocate memory";
> -			goto out_put_sb_page;
> -		}
> -
> 		mutex_lock(&bch_register_lock);
> -		ret = register_bdev(sb, sb_disk, bdev, dc);
> +		ret = register_bdev(sb, sb_disk, bdev, holder);
> 		mutex_unlock(&bch_register_lock);
> 		/* blkdev_put() will be called in cached_dev_free() */
> 		if (ret < 0)
> 			goto out_free_sb;
> 	} else {
> -		struct cache *ca = kzalloc(sizeof(*ca), GFP_KERNEL);
> -
> -		if (!ca) {
> -			ret = -ENOMEM;
> -			err = "cannot allocate memory";
> -			goto out_put_sb_page;
> -		}
> -
> 		/* blkdev_put() will be called in bch_cache_release() */
> -		ret = register_cache(sb, sb_disk, bdev, ca);
> +		ret = register_cache(sb, sb_disk, bdev, holder);
> 		if (ret)
> 			goto out_free_sb;
> 	}
> @@ -2644,6 +2626,8 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
> async_done:
> 	return size;
> 
> +out_free_holder:
> +	kfree(holder);
> out_put_sb_page:
> 	put_page(virt_to_page(sb_disk));
> out_blkdev_put:
> -- 
> 2.35.3
> 

-- 
Coly Li

[PATCH v2 2/2] bcache: Fix bcache device claiming

[Jan Kara]

Commit 2736e8eeb0cc ("block: use the holder as indication for exclusive
opens") introduced a change that blkdev_put() has to get exclusive
holder of the bdev as an argument. However it overlooked that
register_bdev() and register_cache() overwrite the bdev->bd_holder field
in the block device to point to the real owning object which was not
available at the time we called blkdev_get_by_path(). Messing with bdev
internals like this is a layering violation and it also causes
blkdev_put() to issue warning about mismatching holders.

Fix bcache to reopen the block device with appropriate holder once it is
available which also restores the behavior that multiple bcache caches
cannot claim the same device which was broken by commit 29499ab060fe
("bcache: don't pass a stack address to blkdev_get_by_path").

Fixes: 2736e8eeb0cc ("block: use the holder as indication for exclusive opens")
Signed-off-by: Jan Kara <jack@suse.cz>
---
 drivers/md/bcache/super.c | 65 +++++++++++++++++++++++----------------
 1 file changed, 38 insertions(+), 27 deletions(-)

diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index 913dd94353b6..0ae2b3676293 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -1369,7 +1369,7 @@ static void cached_dev_free(struct closure *cl)
 		put_page(virt_to_page(dc->sb_disk));
 
 	if (!IS_ERR_OR_NULL(dc->bdev))
-		blkdev_put(dc->bdev, bcache_kobj);
+		blkdev_put(dc->bdev, dc);
 
 	wake_up(&unregister_wait);
 
@@ -1453,7 +1453,6 @@ static int register_bdev(struct cache_sb *sb, struct cache_sb_disk *sb_disk,
 
 	memcpy(&dc->sb, sb, sizeof(struct cache_sb));
 	dc->bdev = bdev;
-	dc->bdev->bd_holder = dc;
 	dc->sb_disk = sb_disk;
 
 	if (cached_dev_init(dc, sb->block_size << 9))
@@ -2218,7 +2217,7 @@ void bch_cache_release(struct kobject *kobj)
 		put_page(virt_to_page(ca->sb_disk));
 
 	if (!IS_ERR_OR_NULL(ca->bdev))
-		blkdev_put(ca->bdev, bcache_kobj);
+		blkdev_put(ca->bdev, ca);
 
 	kfree(ca);
 	module_put(THIS_MODULE);
@@ -2345,7 +2344,6 @@ static int register_cache(struct cache_sb *sb, struct cache_sb_disk *sb_disk,
 
 	memcpy(&ca->sb, sb, sizeof(struct cache_sb));
 	ca->bdev = bdev;
-	ca->bdev->bd_holder = ca;
 	ca->sb_disk = sb_disk;
 
 	if (bdev_max_discard_sectors((bdev)))
@@ -2359,7 +2357,7 @@ static int register_cache(struct cache_sb *sb, struct cache_sb_disk *sb_disk,
 		 * call blkdev_put() to bdev in bch_cache_release(). So we
 		 * explicitly call blkdev_put() here.
 		 */
-		blkdev_put(bdev, bcache_kobj);
+		blkdev_put(bdev, ca);
 		if (ret == -ENOMEM)
 			err = "cache_alloc(): -ENOMEM";
 		else if (ret == -EPERM)
@@ -2516,10 +2514,11 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
 	char *path = NULL;
 	struct cache_sb *sb;
 	struct cache_sb_disk *sb_disk;
-	struct block_device *bdev;
-	void *holder;
+	struct block_device *bdev, *bdev2;
+	void *holder = NULL;
 	ssize_t ret;
 	bool async_registration = false;
+	bool quiet = false;
 
 #ifdef CONFIG_BCACHE_ASYNC_REGISTRATION
 	async_registration = true;
@@ -2548,24 +2547,9 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
 
 	ret = -EINVAL;
 	err = "failed to open device";
-	bdev = blkdev_get_by_path(strim(path), BLK_OPEN_READ | BLK_OPEN_WRITE,
-				  bcache_kobj, NULL);
-	if (IS_ERR(bdev)) {
-		if (bdev == ERR_PTR(-EBUSY)) {
-			dev_t dev;
-
-			mutex_lock(&bch_register_lock);
-			if (lookup_bdev(strim(path), &dev) == 0 &&
-			    bch_is_open(dev))
-				err = "device already registered";
-			else
-				err = "device busy";
-			mutex_unlock(&bch_register_lock);
-			if (attr == &ksysfs_register_quiet)
-				goto done;
-		}
+	bdev = blkdev_get_by_path(strim(path), BLK_OPEN_READ, NULL, NULL);
+	if (IS_ERR(bdev))
 		goto out_free_sb;
-	}
 
 	err = "failed to set blocksize";
 	if (set_blocksize(bdev, 4096))
@@ -2582,6 +2566,32 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
 		goto out_put_sb_page;
 	}
 
+	/* Now reopen in exclusive mode with proper holder */
+	bdev2 = blkdev_get_by_dev(bdev->bd_dev, BLK_OPEN_READ | BLK_OPEN_WRITE,
+				  holder, NULL);
+	blkdev_put(bdev, NULL);
+	bdev = bdev2;
+	if (IS_ERR(bdev)) {
+		ret = PTR_ERR(bdev);
+		bdev = NULL;
+		if (ret == -EBUSY) {
+			dev_t dev;
+
+			mutex_lock(&bch_register_lock);
+			if (lookup_bdev(strim(path), &dev) == 0 &&
+			    bch_is_open(dev))
+				err = "device already registered";
+			else
+				err = "device busy";
+			mutex_unlock(&bch_register_lock);
+			if (attr == &ksysfs_register_quiet) {
+				quiet = true;
+				ret = size;
+			}
+		}
+		goto out_free_holder;
+	}
+
 	err = "failed to register device";
 
 	if (async_registration) {
@@ -2619,7 +2629,6 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
 			goto out_free_sb;
 	}
 
-done:
 	kfree(sb);
 	kfree(path);
 	module_put(THIS_MODULE);
@@ -2631,7 +2640,8 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
 out_put_sb_page:
 	put_page(virt_to_page(sb_disk));
 out_blkdev_put:
-	blkdev_put(bdev, register_bcache);
+	if (bdev)
+		blkdev_put(bdev, holder);
 out_free_sb:
 	kfree(sb);
 out_free_path:
@@ -2640,7 +2650,8 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
 out_module_put:
 	module_put(THIS_MODULE);
 out:
-	pr_info("error %s: %s\n", path?path:"", err);
+	if (!quiet)
+		pr_info("error %s: %s\n", path?path:"", err);
 	return ret;
 }
 
-- 
2.35.3

Re: [PATCH v2 2/2] bcache: Fix bcache device claiming

[Kent Overstreet]

On Thu, Jun 22, 2023 at 06:46:55PM +0200, Jan Kara wrote:
> Commit 2736e8eeb0cc ("block: use the holder as indication for exclusive
> opens") introduced a change that blkdev_put() has to get exclusive
> holder of the bdev as an argument. However it overlooked that
> register_bdev() and register_cache() overwrite the bdev->bd_holder field
> in the block device to point to the real owning object which was not
> available at the time we called blkdev_get_by_path(). Messing with bdev
> internals like this is a layering violation and it also causes
> blkdev_put() to issue warning about mismatching holders.
> 
> Fix bcache to reopen the block device with appropriate holder once it is
> available which also restores the behavior that multiple bcache caches
> cannot claim the same device which was broken by commit 29499ab060fe
> ("bcache: don't pass a stack address to blkdev_get_by_path").
> 
> Fixes: 2736e8eeb0cc ("block: use the holder as indication for exclusive opens")
> Signed-off-by: Jan Kara <jack@suse.cz>

Reviewed-by: Kent Overstreet <kent.overstreet@linux.dev>

> ---
>  drivers/md/bcache/super.c | 65 +++++++++++++++++++++++----------------
>  1 file changed, 38 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index 913dd94353b6..0ae2b3676293 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -1369,7 +1369,7 @@ static void cached_dev_free(struct closure *cl)
>  		put_page(virt_to_page(dc->sb_disk));
>  
>  	if (!IS_ERR_OR_NULL(dc->bdev))
> -		blkdev_put(dc->bdev, bcache_kobj);
> +		blkdev_put(dc->bdev, dc);
>  
>  	wake_up(&unregister_wait);
>  
> @@ -1453,7 +1453,6 @@ static int register_bdev(struct cache_sb *sb, struct cache_sb_disk *sb_disk,
>  
>  	memcpy(&dc->sb, sb, sizeof(struct cache_sb));
>  	dc->bdev = bdev;
> -	dc->bdev->bd_holder = dc;
>  	dc->sb_disk = sb_disk;
>  
>  	if (cached_dev_init(dc, sb->block_size << 9))
> @@ -2218,7 +2217,7 @@ void bch_cache_release(struct kobject *kobj)
>  		put_page(virt_to_page(ca->sb_disk));
>  
>  	if (!IS_ERR_OR_NULL(ca->bdev))
> -		blkdev_put(ca->bdev, bcache_kobj);
> +		blkdev_put(ca->bdev, ca);
>  
>  	kfree(ca);
>  	module_put(THIS_MODULE);
> @@ -2345,7 +2344,6 @@ static int register_cache(struct cache_sb *sb, struct cache_sb_disk *sb_disk,
>  
>  	memcpy(&ca->sb, sb, sizeof(struct cache_sb));
>  	ca->bdev = bdev;
> -	ca->bdev->bd_holder = ca;
>  	ca->sb_disk = sb_disk;
>  
>  	if (bdev_max_discard_sectors((bdev)))
> @@ -2359,7 +2357,7 @@ static int register_cache(struct cache_sb *sb, struct cache_sb_disk *sb_disk,
>  		 * call blkdev_put() to bdev in bch_cache_release(). So we
>  		 * explicitly call blkdev_put() here.
>  		 */
> -		blkdev_put(bdev, bcache_kobj);
> +		blkdev_put(bdev, ca);
>  		if (ret == -ENOMEM)
>  			err = "cache_alloc(): -ENOMEM";
>  		else if (ret == -EPERM)
> @@ -2516,10 +2514,11 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
>  	char *path = NULL;
>  	struct cache_sb *sb;
>  	struct cache_sb_disk *sb_disk;
> -	struct block_device *bdev;
> -	void *holder;
> +	struct block_device *bdev, *bdev2;
> +	void *holder = NULL;
>  	ssize_t ret;
>  	bool async_registration = false;
> +	bool quiet = false;
>  
>  #ifdef CONFIG_BCACHE_ASYNC_REGISTRATION
>  	async_registration = true;
> @@ -2548,24 +2547,9 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
>  
>  	ret = -EINVAL;
>  	err = "failed to open device";
> -	bdev = blkdev_get_by_path(strim(path), BLK_OPEN_READ | BLK_OPEN_WRITE,
> -				  bcache_kobj, NULL);
> -	if (IS_ERR(bdev)) {
> -		if (bdev == ERR_PTR(-EBUSY)) {
> -			dev_t dev;
> -
> -			mutex_lock(&bch_register_lock);
> -			if (lookup_bdev(strim(path), &dev) == 0 &&
> -			    bch_is_open(dev))
> -				err = "device already registered";
> -			else
> -				err = "device busy";
> -			mutex_unlock(&bch_register_lock);
> -			if (attr == &ksysfs_register_quiet)
> -				goto done;
> -		}
> +	bdev = blkdev_get_by_path(strim(path), BLK_OPEN_READ, NULL, NULL);
> +	if (IS_ERR(bdev))
>  		goto out_free_sb;
> -	}
>  
>  	err = "failed to set blocksize";
>  	if (set_blocksize(bdev, 4096))
> @@ -2582,6 +2566,32 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
>  		goto out_put_sb_page;
>  	}
>  
> +	/* Now reopen in exclusive mode with proper holder */
> +	bdev2 = blkdev_get_by_dev(bdev->bd_dev, BLK_OPEN_READ | BLK_OPEN_WRITE,
> +				  holder, NULL);
> +	blkdev_put(bdev, NULL);
> +	bdev = bdev2;
> +	if (IS_ERR(bdev)) {
> +		ret = PTR_ERR(bdev);
> +		bdev = NULL;
> +		if (ret == -EBUSY) {
> +			dev_t dev;
> +
> +			mutex_lock(&bch_register_lock);
> +			if (lookup_bdev(strim(path), &dev) == 0 &&
> +			    bch_is_open(dev))
> +				err = "device already registered";
> +			else
> +				err = "device busy";
> +			mutex_unlock(&bch_register_lock);
> +			if (attr == &ksysfs_register_quiet) {
> +				quiet = true;
> +				ret = size;
> +			}
> +		}
> +		goto out_free_holder;
> +	}
> +
>  	err = "failed to register device";
>  
>  	if (async_registration) {
> @@ -2619,7 +2629,6 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
>  			goto out_free_sb;
>  	}
>  
> -done:
>  	kfree(sb);
>  	kfree(path);
>  	module_put(THIS_MODULE);
> @@ -2631,7 +2640,8 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
>  out_put_sb_page:
>  	put_page(virt_to_page(sb_disk));
>  out_blkdev_put:
> -	blkdev_put(bdev, register_bcache);
> +	if (bdev)
> +		blkdev_put(bdev, holder);
>  out_free_sb:
>  	kfree(sb);
>  out_free_path:
> @@ -2640,7 +2650,8 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
>  out_module_put:
>  	module_put(THIS_MODULE);
>  out:
> -	pr_info("error %s: %s\n", path?path:"", err);
> +	if (!quiet)
> +		pr_info("error %s: %s\n", path?path:"", err);
>  	return ret;
>  }
>  
> -- 
> 2.35.3
> 

Re: [PATCH v2 2/2] bcache: Fix bcache device claiming

[Coly Li]

On Thu, Jun 22, 2023 at 06:46:55PM +0200, Jan Kara wrote:
> Commit 2736e8eeb0cc ("block: use the holder as indication for exclusive
> opens") introduced a change that blkdev_put() has to get exclusive
> holder of the bdev as an argument. However it overlooked that
> register_bdev() and register_cache() overwrite the bdev->bd_holder field
> in the block device to point to the real owning object which was not
> available at the time we called blkdev_get_by_path(). Messing with bdev
> internals like this is a layering violation and it also causes
> blkdev_put() to issue warning about mismatching holders.
> 
> Fix bcache to reopen the block device with appropriate holder once it is
> available which also restores the behavior that multiple bcache caches
> cannot claim the same device which was broken by commit 29499ab060fe
> ("bcache: don't pass a stack address to blkdev_get_by_path").
> 
> Fixes: 2736e8eeb0cc ("block: use the holder as indication for exclusive opens")
> Signed-off-by: Jan Kara <jack@suse.cz>

Acked-by: Coly Li <colyli@suse.de>

Thanks.

Coly Li

> ---
> drivers/md/bcache/super.c | 65 +++++++++++++++++++++++----------------
> 1 file changed, 38 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index 913dd94353b6..0ae2b3676293 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -1369,7 +1369,7 @@ static void cached_dev_free(struct closure *cl)
> 		put_page(virt_to_page(dc->sb_disk));
> 
> 	if (!IS_ERR_OR_NULL(dc->bdev))
> -		blkdev_put(dc->bdev, bcache_kobj);
> +		blkdev_put(dc->bdev, dc);
> 
> 	wake_up(&unregister_wait);
> 
> @@ -1453,7 +1453,6 @@ static int register_bdev(struct cache_sb *sb, struct cache_sb_disk *sb_disk,
> 
> 	memcpy(&dc->sb, sb, sizeof(struct cache_sb));
> 	dc->bdev = bdev;
> -	dc->bdev->bd_holder = dc;
> 	dc->sb_disk = sb_disk;
> 
> 	if (cached_dev_init(dc, sb->block_size << 9))
> @@ -2218,7 +2217,7 @@ void bch_cache_release(struct kobject *kobj)
> 		put_page(virt_to_page(ca->sb_disk));
> 
> 	if (!IS_ERR_OR_NULL(ca->bdev))
> -		blkdev_put(ca->bdev, bcache_kobj);
> +		blkdev_put(ca->bdev, ca);
> 
> 	kfree(ca);
> 	module_put(THIS_MODULE);
> @@ -2345,7 +2344,6 @@ static int register_cache(struct cache_sb *sb, struct cache_sb_disk *sb_disk,
> 
> 	memcpy(&ca->sb, sb, sizeof(struct cache_sb));
> 	ca->bdev = bdev;
> -	ca->bdev->bd_holder = ca;
> 	ca->sb_disk = sb_disk;
> 
> 	if (bdev_max_discard_sectors((bdev)))
> @@ -2359,7 +2357,7 @@ static int register_cache(struct cache_sb *sb, struct cache_sb_disk *sb_disk,
> 		 * call blkdev_put() to bdev in bch_cache_release(). So we
> 		 * explicitly call blkdev_put() here.
> 		 */
> -		blkdev_put(bdev, bcache_kobj);
> +		blkdev_put(bdev, ca);
> 		if (ret == -ENOMEM)
> 			err = "cache_alloc(): -ENOMEM";
> 		else if (ret == -EPERM)
> @@ -2516,10 +2514,11 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
> 	char *path = NULL;
> 	struct cache_sb *sb;
> 	struct cache_sb_disk *sb_disk;
> -	struct block_device *bdev;
> -	void *holder;
> +	struct block_device *bdev, *bdev2;
> +	void *holder = NULL;
> 	ssize_t ret;
> 	bool async_registration = false;
> +	bool quiet = false;
> 
> #ifdef CONFIG_BCACHE_ASYNC_REGISTRATION
> 	async_registration = true;
> @@ -2548,24 +2547,9 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
> 
> 	ret = -EINVAL;
> 	err = "failed to open device";
> -	bdev = blkdev_get_by_path(strim(path), BLK_OPEN_READ | BLK_OPEN_WRITE,
> -				  bcache_kobj, NULL);
> -	if (IS_ERR(bdev)) {
> -		if (bdev == ERR_PTR(-EBUSY)) {
> -			dev_t dev;
> -
> -			mutex_lock(&bch_register_lock);
> -			if (lookup_bdev(strim(path), &dev) == 0 &&
> -			    bch_is_open(dev))
> -				err = "device already registered";
> -			else
> -				err = "device busy";
> -			mutex_unlock(&bch_register_lock);
> -			if (attr == &ksysfs_register_quiet)
> -				goto done;
> -		}
> +	bdev = blkdev_get_by_path(strim(path), BLK_OPEN_READ, NULL, NULL);
> +	if (IS_ERR(bdev))
> 		goto out_free_sb;
> -	}
> 
> 	err = "failed to set blocksize";
> 	if (set_blocksize(bdev, 4096))
> @@ -2582,6 +2566,32 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
> 		goto out_put_sb_page;
> 	}
> 
> +	/* Now reopen in exclusive mode with proper holder */
> +	bdev2 = blkdev_get_by_dev(bdev->bd_dev, BLK_OPEN_READ | BLK_OPEN_WRITE,
> +				  holder, NULL);
> +	blkdev_put(bdev, NULL);
> +	bdev = bdev2;
> +	if (IS_ERR(bdev)) {
> +		ret = PTR_ERR(bdev);
> +		bdev = NULL;
> +		if (ret == -EBUSY) {
> +			dev_t dev;
> +
> +			mutex_lock(&bch_register_lock);
> +			if (lookup_bdev(strim(path), &dev) == 0 &&
> +			    bch_is_open(dev))
> +				err = "device already registered";
> +			else
> +				err = "device busy";
> +			mutex_unlock(&bch_register_lock);
> +			if (attr == &ksysfs_register_quiet) {
> +				quiet = true;
> +				ret = size;
> +			}
> +		}
> +		goto out_free_holder;
> +	}
> +
> 	err = "failed to register device";
> 
> 	if (async_registration) {
> @@ -2619,7 +2629,6 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
> 			goto out_free_sb;
> 	}
> 
> -done:
> 	kfree(sb);
> 	kfree(path);
> 	module_put(THIS_MODULE);
> @@ -2631,7 +2640,8 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
> out_put_sb_page:
> 	put_page(virt_to_page(sb_disk));
> out_blkdev_put:
> -	blkdev_put(bdev, register_bcache);
> +	if (bdev)
> +		blkdev_put(bdev, holder);
> out_free_sb:
> 	kfree(sb);
> out_free_path:
> @@ -2640,7 +2650,8 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
> out_module_put:
> 	module_put(THIS_MODULE);
> out:
> -	pr_info("error %s: %s\n", path?path:"", err);
> +	if (!quiet)
> +		pr_info("error %s: %s\n", path?path:"", err);
> 	return ret;
> }
> 
> -- 
> 2.35.3
> 

-- 
Coly Li

Re: [PATCH v2 2/2] bcache: Fix bcache device claiming

[Coly Li]

On Thu, Jun 22, 2023 at 06:46:55PM +0200, Jan Kara wrote:
> Commit 2736e8eeb0cc ("block: use the holder as indication for exclusive
> opens") introduced a change that blkdev_put() has to get exclusive
> holder of the bdev as an argument. However it overlooked that
> register_bdev() and register_cache() overwrite the bdev->bd_holder field
> in the block device to point to the real owning object which was not
> available at the time we called blkdev_get_by_path(). Messing with bdev
> internals like this is a layering violation and it also causes
> blkdev_put() to issue warning about mismatching holders.
> 
> Fix bcache to reopen the block device with appropriate holder once it is
> available which also restores the behavior that multiple bcache caches
> cannot claim the same device which was broken by commit 29499ab060fe
> ("bcache: don't pass a stack address to blkdev_get_by_path").
> 
> Fixes: 2736e8eeb0cc ("block: use the holder as indication for exclusive opens")
> Signed-off-by: Jan Kara <jack@suse.cz>

Acked-by: Coly Li <colyli@suse.de>

Thanks.

Coly Li

> ---
>  drivers/md/bcache/super.c | 65 +++++++++++++++++++++++----------------
>  1 file changed, 38 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index 913dd94353b6..0ae2b3676293 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -1369,7 +1369,7 @@ static void cached_dev_free(struct closure *cl)
>  		put_page(virt_to_page(dc->sb_disk));
>  
>  	if (!IS_ERR_OR_NULL(dc->bdev))
> -		blkdev_put(dc->bdev, bcache_kobj);
> +		blkdev_put(dc->bdev, dc);
>  
>  	wake_up(&unregister_wait);
>  
> @@ -1453,7 +1453,6 @@ static int register_bdev(struct cache_sb *sb, struct cache_sb_disk *sb_disk,
>  
>  	memcpy(&dc->sb, sb, sizeof(struct cache_sb));
>  	dc->bdev = bdev;
> -	dc->bdev->bd_holder = dc;
>  	dc->sb_disk = sb_disk;
>  
>  	if (cached_dev_init(dc, sb->block_size << 9))
> @@ -2218,7 +2217,7 @@ void bch_cache_release(struct kobject *kobj)
>  		put_page(virt_to_page(ca->sb_disk));
>  
>  	if (!IS_ERR_OR_NULL(ca->bdev))
> -		blkdev_put(ca->bdev, bcache_kobj);
> +		blkdev_put(ca->bdev, ca);
>  
>  	kfree(ca);
>  	module_put(THIS_MODULE);
> @@ -2345,7 +2344,6 @@ static int register_cache(struct cache_sb *sb, struct cache_sb_disk *sb_disk,
>  
>  	memcpy(&ca->sb, sb, sizeof(struct cache_sb));
>  	ca->bdev = bdev;
> -	ca->bdev->bd_holder = ca;
>  	ca->sb_disk = sb_disk;
>  
>  	if (bdev_max_discard_sectors((bdev)))
> @@ -2359,7 +2357,7 @@ static int register_cache(struct cache_sb *sb, struct cache_sb_disk *sb_disk,
>  		 * call blkdev_put() to bdev in bch_cache_release(). So we
>  		 * explicitly call blkdev_put() here.
>  		 */
> -		blkdev_put(bdev, bcache_kobj);
> +		blkdev_put(bdev, ca);
>  		if (ret == -ENOMEM)
>  			err = "cache_alloc(): -ENOMEM";
>  		else if (ret == -EPERM)
> @@ -2516,10 +2514,11 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
>  	char *path = NULL;
>  	struct cache_sb *sb;
>  	struct cache_sb_disk *sb_disk;
> -	struct block_device *bdev;
> -	void *holder;
> +	struct block_device *bdev, *bdev2;
> +	void *holder = NULL;
>  	ssize_t ret;
>  	bool async_registration = false;
> +	bool quiet = false;
>  
>  #ifdef CONFIG_BCACHE_ASYNC_REGISTRATION
>  	async_registration = true;
> @@ -2548,24 +2547,9 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
>  
>  	ret = -EINVAL;
>  	err = "failed to open device";
> -	bdev = blkdev_get_by_path(strim(path), BLK_OPEN_READ | BLK_OPEN_WRITE,
> -				  bcache_kobj, NULL);
> -	if (IS_ERR(bdev)) {
> -		if (bdev == ERR_PTR(-EBUSY)) {
> -			dev_t dev;
> -
> -			mutex_lock(&bch_register_lock);
> -			if (lookup_bdev(strim(path), &dev) == 0 &&
> -			    bch_is_open(dev))
> -				err = "device already registered";
> -			else
> -				err = "device busy";
> -			mutex_unlock(&bch_register_lock);
> -			if (attr == &ksysfs_register_quiet)
> -				goto done;
> -		}
> +	bdev = blkdev_get_by_path(strim(path), BLK_OPEN_READ, NULL, NULL);
> +	if (IS_ERR(bdev))
>  		goto out_free_sb;
> -	}
>  
>  	err = "failed to set blocksize";
>  	if (set_blocksize(bdev, 4096))
> @@ -2582,6 +2566,32 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
>  		goto out_put_sb_page;
>  	}
>  
> +	/* Now reopen in exclusive mode with proper holder */
> +	bdev2 = blkdev_get_by_dev(bdev->bd_dev, BLK_OPEN_READ | BLK_OPEN_WRITE,
> +				  holder, NULL);
> +	blkdev_put(bdev, NULL);
> +	bdev = bdev2;
> +	if (IS_ERR(bdev)) {
> +		ret = PTR_ERR(bdev);
> +		bdev = NULL;
> +		if (ret == -EBUSY) {
> +			dev_t dev;
> +
> +			mutex_lock(&bch_register_lock);
> +			if (lookup_bdev(strim(path), &dev) == 0 &&
> +			    bch_is_open(dev))
> +				err = "device already registered";
> +			else
> +				err = "device busy";
> +			mutex_unlock(&bch_register_lock);
> +			if (attr == &ksysfs_register_quiet) {
> +				quiet = true;
> +				ret = size;
> +			}
> +		}
> +		goto out_free_holder;
> +	}
> +
>  	err = "failed to register device";
>  
>  	if (async_registration) {
> @@ -2619,7 +2629,6 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
>  			goto out_free_sb;
>  	}
>  
> -done:
>  	kfree(sb);
>  	kfree(path);
>  	module_put(THIS_MODULE);
> @@ -2631,7 +2640,8 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
>  out_put_sb_page:
>  	put_page(virt_to_page(sb_disk));
>  out_blkdev_put:
> -	blkdev_put(bdev, register_bcache);
> +	if (bdev)
> +		blkdev_put(bdev, holder);
>  out_free_sb:
>  	kfree(sb);
>  out_free_path:
> @@ -2640,7 +2650,8 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
>  out_module_put:
>  	module_put(THIS_MODULE);
>  out:
> -	pr_info("error %s: %s\n", path?path:"", err);
> +	if (!quiet)
> +		pr_info("error %s: %s\n", path?path:"", err);
>  	return ret;
>  }
>  
> -- 
> 2.35.3
> 

-- 
Coly Li

Re: [PATCH v2 0/2] bcache: Fix block device claiming

[Jens Axboe]

On Thu, 22 Jun 2023 18:46:53 +0200, Jan Kara wrote:
> these two patches fix block device claiming for bcache broken by recent
> Christoph's changes to blkdev_get_*() functions and also cleans up the layering
> violation inside bcache which was the underlying cause of the breakage.
> 
> This time I've actually tested various modified error handling paths (which was
> good because one of them was indeed wrong!).
> 
> [...]

Applied, thanks!

[1/2] bcache: Alloc holder object before async registration
      commit: abcc0cbd49283fccd20420e86416b2475b00819c
[2/2] bcache: Fix bcache device claiming
      commit: 2c5555983bd27d24162534b682b10654639a5576

Best regards,
-- 
Jens Axboe