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

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

 



On Wed, Jun 21, 2023 at 06:23:26PM +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.

This is a pretty big change for this fix, and we'd want to retest the
error paths - that's hard to do, because the fault injection framework I
was using for that never made it upstream.

What about just exposing a proper API for changing the holder? Wouldn't
that be simpler?




> 
> Signed-off-by: Jan Kara <jack@xxxxxxx>
> ---
>  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
> 



[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