[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] migration/block: optimize the performance by co
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH] migration/block: optimize the performance by coalescing the same write type |
Date: |
Mon, 8 May 2017 17:16:15 -0400 |
User-agent: |
Mutt/1.8.0 (2017-02-23) |
On Mon, Apr 24, 2017 at 09:55:40PM +0800, address@hidden wrote:
> From: Lidong Chen <address@hidden>
>
> This patch optimizes the performance by coalescing the same write type.
> When the zero/non-zero state changes, perform the write for the accumulated
> cluster count.
>
> Signed-off-by: Lidong Chen <address@hidden>
> ---
> Thanks Fam Zheng and Stefan's advice.
> ---
> migration/block.c | 66
> +++++++++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 49 insertions(+), 17 deletions(-)
>
> diff --git a/migration/block.c b/migration/block.c
> index 060087f..e9c5e21 100644
> --- a/migration/block.c
> +++ b/migration/block.c
> @@ -40,6 +40,8 @@
>
> #define MAX_INFLIGHT_IO 512
>
> +#define MIN_CLUSTER_SIZE 8192
The loop accumulates adjacent zero/non-zero regions. What is the point
of this constant?
I guess you are trying to reduce the number of loop iterations when
cluster size is small (e.g. 512). Do you have performance results
showing that this really makes a difference?
It would be simpler to remove MIN_CLUSTER_SIZE. Unless there is
evidence showing it's necessary simpler code is better.
> @@ -943,29 +946,58 @@ static int block_load(QEMUFile *f, void *opaque, int
> version_id)
> nr_sectors * BDRV_SECTOR_SIZE,
> BDRV_REQ_MAY_UNMAP);
> } else {
> - int i;
> - int64_t cur_addr;
> - uint8_t *cur_buf;
> + int64_t cur_addr = addr * BDRV_SECTOR_SIZE;
> + uint8_t *cur_buf = NULL;
> + int64_t last_addr = addr * BDRV_SECTOR_SIZE;
> + uint8_t *last_buf = NULL;
> + int64_t end_addr = addr * BDRV_SECTOR_SIZE + BLOCK_SIZE;
>
> buf = g_malloc(BLOCK_SIZE);
> qemu_get_buffer(f, buf, BLOCK_SIZE);
> - for (i = 0; i < BLOCK_SIZE / cluster_size; i++) {
> - cur_addr = addr * BDRV_SECTOR_SIZE + i * cluster_size;
> - cur_buf = buf + i * cluster_size;
> -
> - if ((!block_mig_state.zero_blocks ||
> - cluster_size < BLOCK_SIZE) &&
> - buffer_is_zero(cur_buf, cluster_size)) {
> - ret = blk_pwrite_zeroes(blk, cur_addr,
> - cluster_size,
> + cur_buf = buf;
> + last_buf = buf;
> +
> + while (last_addr < end_addr) {
> + int is_zero = 0;
Please use bool for boolean values.
signature.asc
Description: PGP signature