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

From: Markus Pargmann
Date: Wed Jul 20 2016 - 03:47:27 EST


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@xxxxxxxxx>
> ---
> 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: signature.asc
Description: This is a digitally signed message part.