Re: [PATCH v5 3/4] make nbd device wait for its users

[Markus Pargmann <mpa@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 4479 bytes --]

Hi,

On Saturday 16 July 2016 16:06:36 Pranay Kr Srivastava wrote:
> 	When a timeout occurs or a recv fails, then
> 	instead of abruptly killing nbd block device
> 	wait for its users to finish.
> 
> 	This is more required when filesystem(s) like
> 	ext2 or ext3 don't expect their buffer heads to
> 	disappear while the filesystem is mounted.
> 
> 	Each open is now refcounted with the device being released
> 	for re-use only when there are no "other users".
> 
> 	A timedout or a disconnected device, if in use, can't
> 	be used until it has been resetted. The reset happens
> 	when all tasks having this bdev open closes this bdev.
> 
> Behavioral Change:
> 
> 1)	NBD_DO_IT will not wait for the device to be reset. Hence
> 	the nbd-client "may exit" while some other process is using
> 	this device without actually doing the reset on this device
> 	, hence thus making it unusable until all such user space
> 	processes have stopped using this device.
> 
> 2)	There's a window where the nbd-client will not be able to
> 	change / issue ioctls to the nbd device. This is when there's
> 	been a disconnect issued or a timeout has occured, however
> 	there are "other" user processes which currently have this
> 	nbd device opened.

Thanks for the updated patches. I applied all of your patches with some
smaller changes. I will test them later and push them to the git repo
then.

Best Regards,

Markus

> 
> Signed-off-by: Pranay Kr Srivastava <pranjas@gmail.com>
> ---
>  drivers/block/nbd.c | 37 ++++++++++++++++++++++++++++++++-----
>  1 file changed, 32 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index 4919760..fe36280 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -74,6 +74,7 @@ struct nbd_device {
>  	 *This is specifically for calling sock_shutdown, for now.
>  	 */
>  	struct work_struct ws_shutdown;
> +	atomic_t users; /* Users that opened the block device */
>  };
>  
>  #if IS_ENABLED(CONFIG_DEBUG_FS)
> @@ -699,6 +700,9 @@ static void nbd_dev_dbg_close(struct nbd_device *nbd);
>  static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
>  		       unsigned int cmd, unsigned long arg)
>  {
> +	if (nbd->disconnect || nbd->timedout)
> +		return -EBUSY;
> +
>  	switch (cmd) {
>  	case NBD_DISCONNECT: {
>  		struct request sreq;
> @@ -728,7 +732,6 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
>  		nbd_clear_que(nbd);
>  		BUG_ON(!list_empty(&nbd->queue_head));
>  		BUG_ON(!list_empty(&nbd->waiting_queue));
> -		kill_bdev(bdev);
>  		return 0;
>  
>  	case NBD_SET_SOCK: {
> @@ -804,16 +807,12 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
>  		mutex_lock(&nbd->tx_lock);
>  		nbd->task_recv = NULL;
>  		nbd_clear_que(nbd);
> -		kill_bdev(bdev);
> -		nbd_bdev_reset(bdev);
>  
>  		if (nbd->disconnect) /* user requested, ignore socket errors */
>  			error = 0;
>  		if (nbd->timedout)
>  			error = -ETIMEDOUT;
>  
> -		nbd_reset(nbd);
> -
>  		return error;
>  	}
>  
> @@ -852,10 +851,38 @@ static int nbd_ioctl(struct block_device *bdev, fmode_t mode,
>  	return error;
>  }
>  
> +static int nbd_open(struct block_device *bdev, fmode_t mode)
> +{
> +	struct nbd_device *nbd = bdev->bd_disk->private_data;
> +
> +	atomic_inc(&nbd->users);
> +
> +	return 0;
> +}
> +
> +static void nbd_release(struct gendisk *disk, fmode_t mode)
> +{
> +	struct nbd_device *nbd = disk->private_data;
> +	struct block_device *bdev = bdget (part_devt(
> +						dev_to_part(nbd_to_dev(nbd))));
> +
> +	WARN_ON(!bdev);
> +	if (atomic_dec_and_test(&nbd->users)) {
> +		if (bdev) {
> +			nbd_bdev_reset(bdev);
> +			kill_bdev(bdev);
> +			bdput(bdev);
> +		}
> +		nbd_reset(nbd);
> +	}
> +}
> +
>  static const struct block_device_operations nbd_fops = {
>  	.owner =	THIS_MODULE,
>  	.ioctl =	nbd_ioctl,
>  	.compat_ioctl =	nbd_ioctl,
> +	.open =         nbd_open,
> +	.release =      nbd_release,
>  };
>  
>  static void nbd_ws_func_shutdown(struct work_struct *ws_nbd)
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]