[Devel] [PATCH RHEL7 COMMIT] ploop: push_backup: merge pbd->cbt_map back to CBT

[Konstantin Khorenko]

Maxim, please verify the changed hunk.

+ see comment below.

On 05/27/2016 12:47 PM, Konstantin Khorenko wrote:
> The commit is pushed to "branch-rh7-3.10.0-327.18.2.vz7.14.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
> after rh7-3.10.0-327.18.2.vz7.14.8
> ------>
> commit 3bc16bb34d02489fd657d7be70bd77595f1015f9
> Author: Maxim Patlasov <mpatlasov at virtuozzo.com>
> Date:   Fri May 27 13:47:48 2016 +0400
>
>      ploop: push_backup: merge pbd->cbt_map back to CBT
>
>      The patch merges its copy of CBT mask back to CBT when
>      push_backup internal state becomes invalid.
>
>      The only exception is when userspace explicitly asks
>      kernel to release pbd->cbt_map without merge:
>
>      ioctl(PLOOP_IOC_PUSH_BACKUP_STOP) with ctl.status == 0.
>
>      https://jira.sw.ru/browse/PSBM-47429
>
>      Signed-off-by: Maxim Patlasov <mpatlasov at virtuozzo.com>
>
>      v2 changes:
>      khorenko@: ploop_pb_destroy() hunk added instead of ploop_push_backup_stop(),
>      caused by f43eec3 ("ploop: push_backup: factor out destroy").
> ---
>   drivers/block/ploop/dev.c         |  2 +-
>   drivers/block/ploop/push_backup.c | 38 ++++++++++++++++++++++++++++++++------
>   drivers/block/ploop/push_backup.h |  2 +-
>   3 files changed, 34 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/block/ploop/dev.c b/drivers/block/ploop/dev.c
> index 8e3f97f..07764f9 100644
> --- a/drivers/block/ploop/dev.c
> +++ b/drivers/block/ploop/dev.c
> @@ -3808,7 +3808,7 @@ static int ploop_stop(struct ploop_device * plo, struct block_device *bdev)
>   	}
>
>   	clear_bit(PLOOP_S_PUSH_BACKUP, &plo->state);
> -	ploop_pb_stop(plo->pbd);
> +	ploop_pb_stop(plo->pbd, true);
>
>   	for (p = plo->disk->minors - 1; p > 0; p--)
>   		invalidate_partition(plo->disk, p);
> diff --git a/drivers/block/ploop/push_backup.c b/drivers/block/ploop/push_backup.c
> index 16b1a5d..376052d 100644
> --- a/drivers/block/ploop/push_backup.c
> +++ b/drivers/block/ploop/push_backup.c
> @@ -88,6 +88,29 @@ static void ploop_pb_map_free(struct page **map, unsigned long block_max)
>   	}
>   }
>
> +int ploop_pb_cbt_map_release(struct ploop_pushbackup_desc *pbd, bool do_merge)
> +{
> +	int ret = 0;
> +
> +	if (pbd->cbt_map == NULL)
> +		return 0;
> +
> +	if (do_merge) {
> +		ret = blk_cbt_map_merge(pbd->plo->queue,
> +					pbd->cbt_uuid,
> +					pbd->cbt_map,
> +					pbd->cbt_block_max,
> +					pbd->cbt_block_bits);
> +		if (ret)
> +			printk("ploop(%d): blk_cbt_map_merge() failed with "
> +			       "%d\n", pbd->plo->index, ret);
> +	}
> +
> +	ploop_pb_map_free(pbd->cbt_map, pbd->cbt_block_max);
> +	pbd->cbt_map = NULL;
> +	return ret;
> +}
> +
>   struct ploop_pushbackup_desc *ploop_pb_alloc(struct ploop_device *plo)
>   {
>   	struct ploop_pushbackup_desc *pbd;
> @@ -314,7 +337,7 @@ void ploop_pb_fini(struct ploop_pushbackup_desc *pbd)
>   		mutex_unlock(&plo->sysfs_mutex);
>   	}
>
> -	ploop_pb_map_free(pbd->cbt_map, pbd->cbt_block_max);
> +	ploop_pb_cbt_map_release(pbd, true);
>   	ploop_pb_map_free(pbd->ppb_map, pbd->ppb_block_max);
>   	ploop_pb_map_free(pbd->reported_map, pbd->ppb_block_max);
>
> @@ -334,8 +357,6 @@ int ploop_pb_copy_cbt_to_user(struct ploop_pushbackup_desc *pbd, char *user_addr
>   		user_addr += PAGE_SIZE;
>   	}
>
> -	ploop_pb_map_free(pbd->cbt_map, pbd->cbt_block_max);
> -	pbd->cbt_map = NULL;
>   	return 0;
>   }
>
> @@ -496,14 +517,17 @@ int ploop_pb_preq_add_pending(struct ploop_pushbackup_desc *pbd,
>   	return 0;
>   }
>
> -unsigned long ploop_pb_stop(struct ploop_pushbackup_desc *pbd)
> +unsigned long ploop_pb_stop(struct ploop_pushbackup_desc *pbd, bool do_merge)
>   {
>   	unsigned long ret = 0;
> +	int merge_status = 0;
>   	LIST_HEAD(drop_list);
>
>   	if (pbd == NULL)
>   		return 0;
>
> +	merge_status = ploop_pb_cbt_map_release(pbd, do_merge);
> +
>   	spin_lock(&pbd->ppb_lock);
>
>   	while (!RB_EMPTY_ROOT(&pbd->pending_tree)) {
> @@ -535,7 +559,7 @@ unsigned long ploop_pb_stop(struct ploop_pushbackup_desc *pbd)
>   		spin_unlock_irq(&plo->lock);
>   	}
>
> -	return ret;
> +	return merge_status ? : ret;
>   }
>
>   int ploop_pb_get_pending(struct ploop_pushbackup_desc *pbd,
> @@ -682,12 +706,14 @@ int ploop_pb_destroy(struct ploop_device *plo, __u32 *status)
>   {
>   	struct ploop_pushbackup_desc *pbd = plo->pbd;
>   	unsigned long ret;
> +	bool do_merge;
>
>   	if (!test_and_clear_bit(PLOOP_S_PUSH_BACKUP, &plo->state))
>   		return -EINVAL;
>
>   	BUG_ON (!pbd);
> -	ret = ploop_pb_stop(pbd);
> +	do_merge = status ? *status : true;
> +	ret = ploop_pb_stop(pbd, do_merge);
>
>   	if (status)
>   		*status = ret;

Why do we need this *status = ret?
May be we need to return not always 0 here, but "ret" in ploop_pb_destroy()?

> diff --git a/drivers/block/ploop/push_backup.h b/drivers/block/ploop/push_backup.h
> index 1a8636a..1644785 100644
> --- a/drivers/block/ploop/push_backup.h
> +++ b/drivers/block/ploop/push_backup.h
> @@ -4,7 +4,7 @@ struct ploop_pushbackup_desc *ploop_pb_alloc(struct ploop_device *plo);
>   int ploop_pb_init(struct ploop_pushbackup_desc *pbd, __u8 *uuid, bool full);
>   void ploop_pb_fini(struct ploop_pushbackup_desc *pbd);
>   int ploop_pb_copy_cbt_to_user(struct ploop_pushbackup_desc *pbd, char *user_addr);
> -unsigned long ploop_pb_stop(struct ploop_pushbackup_desc *pbd);
> +unsigned long ploop_pb_stop(struct ploop_pushbackup_desc *pbd, bool do_merge);
>   int ploop_pb_check_uuid(struct ploop_pushbackup_desc *pbd, __u8 *uuid);
>   int ploop_pb_get_uuid(struct ploop_pushbackup_desc *pbd, __u8 *uuid);
>
> .
>