[Devel] [PATCH vz7 v2] ploop: increase logging on errors when opening new deltas

[Pavel Tikhomirov]

On 09.06.2023 00:16, Alexander Atanasov wrote:
> Ocassionally we got EBUSY but it is a bit over used,
> so it is not clear what it means.
> 
> Add more logging to catch the source of the error.
> 
> https://jira.vzint.dev/browse/PSBM-146836

Reviewed-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>

> Signed-off-by: Alexander Atanasov <alexander.atanasov at virtuozzo.com>
> ---
>   drivers/block/ploop/dev.c         |  9 ++++++++-
>   drivers/block/ploop/fmt_ploop1.c  |  4 +++-
>   drivers/block/ploop/io_kaio.c     | 13 ++++++++++---
>   drivers/block/ploop/io_kaio_map.c |  7 +++++--
>   4 files changed, 26 insertions(+), 7 deletions(-)
> 
> v1->v2: addressing review comments - print the error value and fix coding style
> 
> diff --git a/drivers/block/ploop/dev.c b/drivers/block/ploop/dev.c
> index 2833865b087f..8e238eafd9f2 100644
> --- a/drivers/block/ploop/dev.c
> +++ b/drivers/block/ploop/dev.c
> @@ -3582,8 +3582,11 @@ static int ploop_replace_delta(struct ploop_device * plo, unsigned long arg)
>   			   sizeof(struct ploop_ctl_chunk)))
>   		return -EFAULT;
>   
> -	if (plo->maintenance_type != PLOOP_MNTN_OFF)
> +	if (plo->maintenance_type != PLOOP_MNTN_OFF) {
> +		if (printk_ratelimit())
> +			PL_WARN(plo, "Attempt to replace while in maintenance mode\n");
>   		return -EBUSY;
> +	}
>   
>   	old_delta = find_delta(plo, ctl.pctl_level);
>   	if (old_delta == NULL)
> @@ -3596,6 +3599,10 @@ static int ploop_replace_delta(struct ploop_device * plo, unsigned long arg)
>   	if (IS_ERR(delta))
>   		return PTR_ERR(delta);
>   
> +	WARN_ONCE(delta->ops != old_delta->ops,
> +		  "New delta uses different io %p vs %p\n",
> +		  delta->ops, old_delta->ops);
> +
>   	err = delta->ops->compose(delta, 1, &chunk);
>   	if (err)
>   		goto out_destroy;
> diff --git a/drivers/block/ploop/fmt_ploop1.c b/drivers/block/ploop/fmt_ploop1.c
> index e59a9eb50ac2..a0369db35c83 100644
> --- a/drivers/block/ploop/fmt_ploop1.c
> +++ b/drivers/block/ploop/fmt_ploop1.c
> @@ -314,8 +314,10 @@ ploop1_open(struct ploop_delta * delta)
>   	if (!(delta->flags & PLOOP_FMT_RDONLY)) {
>   		pvd_header_set_disk_in_use(vh);
>   		err = delta->io.ops->sync_write(&delta->io, ph->dyn_page, 4096, 0, 0);
> -		if (err)
> +		if (err) {
> +			PL_ERR(delta->plo, "failed to update in use err=%d\n", err);
>   			goto out_err;
> +		}
>   	}
>   
>   	delta->io.alloc_head = ph->alloc_head;
> diff --git a/drivers/block/ploop/io_kaio.c b/drivers/block/ploop/io_kaio.c
> index 35c0fad43baf..098f6c7b5f2d 100644
> --- a/drivers/block/ploop/io_kaio.c
> +++ b/drivers/block/ploop/io_kaio.c
> @@ -966,17 +966,23 @@ static int kaio_open(struct ploop_io * io)
>   	io->files.bdev = io->files.inode->i_sb->s_bdev;
>   
>   	err = io->ops->sync(io);
> -	if (err)
> +	if (err) {
> +		PL_WARN(delta->plo, "open failed to sync err=%d\n", err);
>   		return err;
> +	}
>   
>   	mutex_lock(&io->files.inode->i_mutex);
>   	err = kaio_invalidate_cache(io);
> -	if (!err)
> +	if (err)
> +		PL_WARN(delta->plo, "invaldiate_cache failed err=%d\n", err);
> +	else
>   		err = ploop_kaio_open(file, delta->flags & PLOOP_FMT_RDONLY);
>   	mutex_unlock(&io->files.inode->i_mutex);
>   
> -	if (err)
> +	if (err) {
> +		PL_WARN(delta->plo, "open failed err=%d\n", err);
>   		return err;
> +	}
>   
>   	io->files.em_tree = &dummy_em_tree;
>   
> @@ -988,6 +994,7 @@ static int kaio_open(struct ploop_io * io)
>   			err = PTR_ERR(io->fsync_thread);
>   			io->fsync_thread = NULL;
>   			ploop_kaio_close(io->files.mapping, 0);
> +			PL_WARN(delta->plo, "fsync thread start failed=%d\n", err);
>   			return err;
>   		}
>   
> diff --git a/drivers/block/ploop/io_kaio_map.c b/drivers/block/ploop/io_kaio_map.c
> index d4ff39d95e74..59e35c562ef9 100644
> --- a/drivers/block/ploop/io_kaio_map.c
> +++ b/drivers/block/ploop/io_kaio_map.c
> @@ -35,10 +35,13 @@ int ploop_kaio_open(struct file * file, int rdonly)
>   				else
>   					m->readers++;
>   			} else {
> -				if (m->readers)
> +				if (m->readers) {
> +					pr_warn("File is already active:%d\n",
> +						m->readers);
>   					err = -EBUSY;
> -				else
> +				} else {
>   					m->readers = -1;
> +				}
>   			}
>   			goto kaio_open_done;
>   		}

-- 
Best regards, Tikhomirov Pavel
Senior Software Developer, Virtuozzo.