Re: [Devel] [PATCH VZ10 v2 02/10] drivers/md/dm-qcow2: generalize COW index update machinery

[Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>]

> The two-stage L1/L2 entry update pipeline is not COW-specific:
> backward merge already uses it to zero L2 entries, and upcoming
> discard support will be the third user. Rename it to reflect
> what it does.
> 
> No functional changes.
> 
> Feature: dm-qcow2: block device over QCOW2 files driver
> https://virtuozzo.atlassian.net/browse/VSTOR-139406
> Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
>
> diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
> index 2cd8e174049f..08d46eb177dd 100644
> --- a/drivers/md/dm-qcow2-map.c
> +++ b/drivers/md/dm-qcow2-map.c
> @@ -2579,10 +2579,10 @@ static int prepare_l1l2_allocation(struct qcow2 *qcow2, struct qio *qio,
>   * we have to wait all previous READs. We do that around
>   * index wb. See md->wpc_noread_count update details.
>   */
> -static int prepare_l_entry_cow(struct qcow2 *qcow2, struct qcow2_map *map,
> -			       struct qio *qio, struct md_page *md,
> -			       u32 index_in_page, loff_t cow_clu_pos,
> -			       loff_t cow_clu_end, u8 cow_level)
> +static int prepare_l_entry_replace(struct qcow2 *qcow2, struct qcow2_map *map,
> +				   struct qio *qio, struct md_page *md,
> +				   u32 index_in_page, loff_t unuse_clu_pos,
> +				   loff_t unuse_clu_end, u8 lx_level)
>  {
>  	struct lock_desc *lockd = NULL;
>  	struct qio_ext *ext;
> @@ -2591,9 +2591,9 @@ static int prepare_l_entry_cow(struct qcow2 *qcow2, struct qcow2_map *map,
>  		return -ENOMEM;
>  
>  	ext = qio->ext;
> -	ext->cow_clu_pos = cow_clu_pos;
> -	ext->cow_clu_end = cow_clu_end;
> -	ext->cow_level = cow_level;
> +	ext->unuse_clu_pos = unuse_clu_pos;
> +	ext->unuse_clu_end = unuse_clu_end;
> +	ext->lx_level = lx_level;
>  
>  	spin_lock_irq(&qcow2->md_pages_lock);
>  	if (!md->lockd) {
> @@ -2614,23 +2614,23 @@ static int prepare_l_entry_cow(struct qcow2 *qcow2, struct qcow2_map *map,
>  	return 1;
>  }
>  
> -static int prepare_l1l2_cow(struct qcow2 *qcow2, struct qio *qio,
> -			    struct qcow2_map *map)
> +static int prepare_l1l2_replace(struct qcow2 *qcow2, struct qio *qio,
> +				struct qcow2_map *map)
>  {
>  	if (WARN_ON_ONCE(!(map->level & L1_LEVEL)))
>  		return -EIO; /* Sanity check: L1 must be cached */
>  
>  	if (!(map->level & L2_LEVEL)) {
> -		return prepare_l_entry_cow(qcow2, map, qio, map->l1.md,
> -					   map->l1.index_in_page,
> -					   map->cow_clu_pos,
> -					   map->cow_clu_end, L1_LEVEL);
> +		return prepare_l_entry_replace(qcow2, map, qio, map->l1.md,
> +					       map->l1.index_in_page,
> +					       map->cow_clu_pos,
> +					       map->cow_clu_end, L1_LEVEL);
>  	}
>  
> -	return prepare_l_entry_cow(qcow2, map, qio, map->l2.md,
> -				  map->l2.index_in_page,
> -				  map->cow_clu_pos,
> -				  map->cow_clu_end, L2_LEVEL);
> +	return prepare_l_entry_replace(qcow2, map, qio, map->l2.md,
> +				       map->l2.index_in_page,
> +				       map->cow_clu_pos,
> +				       map->cow_clu_end, L2_LEVEL);
>  }
>  
>  static void backward_merge_write_complete(struct qcow2_target *tgt, struct qio *unused,
> @@ -2648,7 +2648,7 @@ static void backward_merge_write_complete(struct qcow2_target *tgt, struct qio *
>  	WARN_ON_ONCE(qio->flags & QIO_IS_DISCARD_FL);
>  	qio->flags |= QIO_IS_DISCARD_FL;
>  
> -	qio->queue_list_id = QLIST_COW_INDEXES;
> +	qio->queue_list_id = QLIST_INDEXES_WRITE;
>  	qcow2_dispatch_qios(qcow2, qio, NULL);
>  }
>  
> @@ -2704,7 +2704,7 @@ static int prepare_backward_merge(struct qcow2 *qcow2, struct qio **qio,
>  	if (!op_is_write((*qio)->bi_op)) {
>  		/*
>  		 * READ qio may data may be contained in several deltas.
> -		 * We can't read lower delta after prepare_l1l2_cow()
> +		 * We can't read lower delta after prepare_l1l2_replace()
>  		 * prepares us.
>  		 */
>  		aux_qio = qcow2_alloc_qio(qcow2->tgt->qio_pool, true);
> @@ -2725,10 +2725,10 @@ static int prepare_backward_merge(struct qcow2 *qcow2, struct qio **qio,
>  	}
>  
>  	/*
> -	 * Mark as COW, as this completely defers any parallel qios.
> -	 * @qio is COW status holder.
> +	 * Lock the entry, as this completely defers any parallel qios.
> +	 * @qio is the lock holder.
>  	 */
> -	ret = prepare_l1l2_cow(qcow2, *qio, map);
> +	ret = prepare_l1l2_replace(qcow2, *qio, map);
>  	if (ret < 0) {
>  		(*qio)->bi_status = errno_to_blk_status(ret);
>  		goto endio;
> @@ -2736,13 +2736,13 @@ static int prepare_backward_merge(struct qcow2 *qcow2, struct qio **qio,
>  
>  	if (!map->clu_is_cow) {
>  		/* Forced set these to unuse them after discard */
> -		(*qio)->ext->cow_clu_pos = map->data_clu_pos;
> -		(*qio)->ext->cow_clu_end = map->data_clu_pos + qcow2->clu_size;
> +		(*qio)->ext->unuse_clu_pos = map->data_clu_pos;
> +		(*qio)->ext->unuse_clu_end = map->data_clu_pos + qcow2->clu_size;
>  	}
>  
>  	return 1;
>  endio:
> -	qio_endio(*qio); /* Breaks COW set in prepare_l1l2_cow() */
> +	qio_endio(*qio); /* Releases the lock set in prepare_l1l2_replace() */
>  	return 0;
>  }
>  
> @@ -3348,7 +3348,7 @@ static int handle_metadata(struct qcow2 *qcow2, struct qio **qio,
>  		   (!qio_is_fully_alloced(qcow2, *qio, map) || map->clu_is_cow)) {
>  		if (map->clu_is_cow) {
>  			/* COW to compressed or shared with snapshot cluster */
> -			ret = prepare_l1l2_cow(qcow2, *qio, map);
> +			ret = prepare_l1l2_replace(qcow2, *qio, map);
>  		} else if ((map->level & L2_LEVEL) &&
>  		    qio_border_is_inside_unmapped_unit(qcow2, *qio, map) &&
>  		    maybe_mapped_in_lower_delta(qcow2, *qio)) {
> @@ -3358,7 +3358,7 @@ static int handle_metadata(struct qcow2 *qcow2, struct qio **qio,
>  			 * snapshots). Here is data COW on L2_LEVEL.
>  			 */
>  			map->backing_file_cow = true;
> -			ret = prepare_l1l2_cow(qcow2, *qio, map);
> +			ret = prepare_l1l2_replace(qcow2, *qio, map);
>  		} else if (unlikely(op_is_discard((*qio)->bi_op) &&
>  				    (map->level & L2_LEVEL))) {
>  			if (!map->data_clu_alloced) {
> @@ -3837,7 +3837,7 @@ static void cow_data_write_endio(struct qcow2_target *tgt, struct qio *unused,
>  		qio->bi_status = bi_status;
>  		qio_endio(qio);
>  	} else {
> -		qio->queue_list_id = QLIST_COW_INDEXES;
> +		qio->queue_list_id = QLIST_INDEXES_WRITE;
>  		qcow2_dispatch_qios(qcow2, qio, NULL);
>  	}
>  }
> @@ -3885,7 +3885,7 @@ static void sliced_cow_data_write_complete(struct qcow2_target *tgt, struct qio
>  		qio->bi_status = bi_status;
>  		qio_endio(qio);
>  	} else {
> -		qio->queue_list_id = QLIST_COW_INDEXES;
> +		qio->queue_list_id = QLIST_INDEXES_WRITE;
>  		qcow2_dispatch_qios(qcow2, qio, NULL);
>  	}
>  }
> @@ -3919,7 +3919,7 @@ static void process_cow_data_write(struct qcow2 *qcow2, struct list_head *cow_li
>  		ext = qio->ext;
>  
>  		if (ext->only_set_ext_l2) {
> -			WARN_ON_ONCE(ext->cow_level != L2_LEVEL);
> +			WARN_ON_ONCE(ext->lx_level != L2_LEVEL);
>  			pos = ext->allocated_clu_pos;
>  			goto submit;
>  		}
> @@ -3938,14 +3938,14 @@ static void process_cow_data_write(struct qcow2 *qcow2, struct list_head *cow_li
>  		ext->allocated_clu_pos = pos;
>  		ext->cleanup_mask |= FREE_ALLOCATED_CLU;
>  submit:
> -		if (ext->cow_level == L2_LEVEL)
> +		if (ext->lx_level == L2_LEVEL)
>  			submit_sliced_cow_data_write(qcow2, qio, pos);
>  		else
>  			submit_cow_data_write(qcow2, qio, pos);
>  	}
>  }
>  
> -static void process_cow_indexes_write(struct qcow2 *qcow2,
> +static void process_indexes_write(struct qcow2 *qcow2,
>  				      struct list_head *qio_list)
>  {
NIT: qvec is unused. Its only set here, so lets just drop it since we touch this
function.

-- 
Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>