diff mbox series

[RFC,5/8] migration/dirtyrate: Compare hash results for recorded ramblock

Message ID 1595646669-109310-6-git-send-email-zhengchuan@huawei.com (mailing list archive)
State New, archived
Headers show
Series *** A Method for evaluating dirty page rate *** | expand

Commit Message

Zheng Chuan July 25, 2020, 3:11 a.m. UTC 
From: Zheng Chuan <zhengchuan@huawei.com>

Compare hash results for recorded ramblock.

Signed-off-by: Zheng Chuan <zhengchuan@huawei.com>
Signed-off-by: YanYing Zhang <ann.zhuangyanying@huawei.com>
---
 migration/dirtyrate.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)

Comments

Dr. David Alan Gilbert Aug. 4, 2020, 5:29 p.m. UTC | #1 
* Chuan Zheng (zhengchuan@huawei.com) wrote:
> From: Zheng Chuan <zhengchuan@huawei.com>
> 
> Compare hash results for recorded ramblock.
> 
> Signed-off-by: Zheng Chuan <zhengchuan@huawei.com>
> Signed-off-by: YanYing Zhang <ann.zhuangyanying@huawei.com>
> ---
>  migration/dirtyrate.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 77 insertions(+)
> 
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index 45cfc91..7badc53 100644
> --- a/migration/dirtyrate.c
> +++ b/migration/dirtyrate.c
> @@ -202,6 +202,83 @@ static int record_block_hash_info(struct dirtyrate_config config,
>      return 0;
>  }
>  
> +static int cal_block_dirty_rate(struct block_dirty_info *info)
> +{
> +    uint8_t *md = NULL;
> +    size_t hash_len;
> +    int i;
> +    int ret = 0;
> +
> +    hash_len = qcrypto_hash_digest_len(QCRYPTO_HASH_ALG_MD5);
> +    md = g_new0(uint8_t, hash_len);

Is 'hash_len' actually constant for a given algorithm, like MD5 ?
i.e. can we just have a nice fixed size array?

> +    for (i = 0; i < info->sample_pages_count; i++) {
> +        ret = get_block_vfn_hash(info, info->sample_page_vfn[i], &md, &hash_len);
> +        if (ret < 0) {
> +            goto out;
> +        }
> +
> +        if (memcmp(md, info->hash_result + i * hash_len, hash_len) != 0) {
> +            info->sample_dirty_count++;

When the page doesn't match, do we have to update info->hash_result with
the new hash?   If the page is only modified once, and we catch it on
this cycle, we wouldn't want to catch it next time around.

> +        }
> +    }
> +
> +out:
> +    g_free(md);
> +    return ret;
> +}
> +
> +static bool find_block_matched(RAMBlock *block, struct block_dirty_info *infos,
> +                               int count, struct block_dirty_info **matched)
> +{
> +    int i;
> +
> +    for (i = 0; i < count; i++) {
> +        if (!strcmp(infos[i].idstr, qemu_ram_get_idstr(block))) {
> +            break;
> +        }
> +    }
> +
> +    if (i == count) {
> +        return false;
> +    }
> +
> +    if (infos[i].block_addr != qemu_ram_get_host_addr(block) ||
> +        infos[i].block_pages !=
> +            (qemu_ram_get_used_length(block) >> DIRTYRATE_PAGE_SIZE_SHIFT)) {

How does this happen?

> +        return false;
> +    }
> +
> +    *matched = &infos[i];
> +    return true;
> +}
> +
> +static int compare_block_hash_info(struct block_dirty_info *info, int block_index)
> +{
> +    struct block_dirty_info *block_dinfo = NULL;
> +    RAMBlock *block = NULL;
> +
> +    RAMBLOCK_FOREACH_MIGRATABLE(block) {
> +        if (ram_block_skip(block) < 0) {
> +            continue;
> +        }
> +        block_dinfo = NULL;
> +        if (!find_block_matched(block, info, block_index + 1, &block_dinfo)) {
> +            continue;
> +        }
> +        if (cal_block_dirty_rate(block_dinfo) < 0) {
> +            return -1;
> +        }
> +        update_dirtyrate_stat(block_dinfo);
> +    }
> +    if (!dirty_stat.total_sample_count) {
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +
>  static void calculate_dirtyrate(struct dirtyrate_config config, int64_t time)
>  {
>      /* todo */
> -- 
> 1.8.3.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Zheng Chuan Aug. 11, 2020, 8:42 a.m. UTC | #2 
On 2020/8/5 1:29, Dr. David Alan Gilbert wrote:
> * Chuan Zheng (zhengchuan@huawei.com) wrote:
>> From: Zheng Chuan <zhengchuan@huawei.com>
>>
>> Compare hash results for recorded ramblock.
>>
>> Signed-off-by: Zheng Chuan <zhengchuan@huawei.com>
>> Signed-off-by: YanYing Zhang <ann.zhuangyanying@huawei.com>
>> ---
>>  migration/dirtyrate.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 77 insertions(+)
>>
>> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
>> index 45cfc91..7badc53 100644
>> --- a/migration/dirtyrate.c
>> +++ b/migration/dirtyrate.c
>> @@ -202,6 +202,83 @@ static int record_block_hash_info(struct dirtyrate_config config,
>>      return 0;
>>  }
>>  
>> +static int cal_block_dirty_rate(struct block_dirty_info *info)
>> +{
>> +    uint8_t *md = NULL;
>> +    size_t hash_len;
>> +    int i;
>> +    int ret = 0;
>> +
>> +    hash_len = qcrypto_hash_digest_len(QCRYPTO_HASH_ALG_MD5);
>> +    md = g_new0(uint8_t, hash_len);
> 
> Is 'hash_len' actually constant for a given algorithm, like MD5 ?
> i.e. can we just have a nice fixed size array?
> 
>> +    for (i = 0; i < info->sample_pages_count; i++) {
>> +        ret = get_block_vfn_hash(info, info->sample_page_vfn[i], &md, &hash_len);
>> +        if (ret < 0) {
>> +            goto out;
>> +        }
>> +
>> +        if (memcmp(md, info->hash_result + i * hash_len, hash_len) != 0) {
>> +            info->sample_dirty_count++;
> 
> When the page doesn't match, do we have to update info->hash_result with
> the new hash?   If the page is only modified once, and we catch it on
> this cycle, we wouldn't want to catch it next time around.
> 
For now, we only support calculate once for each qmp command, thus there is no need
to update it.

However, it is indeed in our plan to add support for calculate multiple times for each qmp command to enhance
dirty rate preciseness:)

>> +        }
>> +    }
>> +
>> +out:
>> +    g_free(md);
>> +    return ret;
>> +}
>> +
>> +static bool find_block_matched(RAMBlock *block, struct block_dirty_info *infos,
>> +                               int count, struct block_dirty_info **matched)
>> +{
>> +    int i;
>> +
>> +    for (i = 0; i < count; i++) {
>> +        if (!strcmp(infos[i].idstr, qemu_ram_get_idstr(block))) {
>> +            break;
>> +        }
>> +    }
>> +
>> +    if (i == count) {
>> +        return false;
>> +    }
>> +
>> +    if (infos[i].block_addr != qemu_ram_get_host_addr(block) ||
>> +        infos[i].block_pages !=
>> +            (qemu_ram_get_used_length(block) >> DIRTYRATE_PAGE_SIZE_SHIFT)) {
> 
> How does this happen?
> 
>> +        return false;
>> +    }
>> +
>> +    *matched = &infos[i];
>> +    return true;
>> +}
>> +
>> +static int compare_block_hash_info(struct block_dirty_info *info, int block_index)
>> +{
>> +    struct block_dirty_info *block_dinfo = NULL;
>> +    RAMBlock *block = NULL;
>> +
>> +    RAMBLOCK_FOREACH_MIGRATABLE(block) {
>> +        if (ram_block_skip(block) < 0) {
>> +            continue;
>> +        }
>> +        block_dinfo = NULL;
>> +        if (!find_block_matched(block, info, block_index + 1, &block_dinfo)) {
>> +            continue;
>> +        }
>> +        if (cal_block_dirty_rate(block_dinfo) < 0) {
>> +            return -1;
>> +        }
>> +        update_dirtyrate_stat(block_dinfo);
>> +    }
>> +    if (!dirty_stat.total_sample_count) {
>> +        return -1;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +
>>  static void calculate_dirtyrate(struct dirtyrate_config config, int64_t time)
>>  {
>>      /* todo */
>> -- 
>> 1.8.3.1
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 
> 
> .
>
diff mbox series

Patch

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index 45cfc91..7badc53 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -202,6 +202,83 @@  static int record_block_hash_info(struct dirtyrate_config config,
     return 0;
 }
 
+static int cal_block_dirty_rate(struct block_dirty_info *info)
+{
+    uint8_t *md = NULL;
+    size_t hash_len;
+    int i;
+    int ret = 0;
+
+    hash_len = qcrypto_hash_digest_len(QCRYPTO_HASH_ALG_MD5);
+    md = g_new0(uint8_t, hash_len);
+
+    for (i = 0; i < info->sample_pages_count; i++) {
+        ret = get_block_vfn_hash(info, info->sample_page_vfn[i], &md, &hash_len);
+        if (ret < 0) {
+            goto out;
+        }
+
+        if (memcmp(md, info->hash_result + i * hash_len, hash_len) != 0) {
+            info->sample_dirty_count++;
+        }
+    }
+
+out:
+    g_free(md);
+    return ret;
+}
+
+static bool find_block_matched(RAMBlock *block, struct block_dirty_info *infos,
+                               int count, struct block_dirty_info **matched)
+{
+    int i;
+
+    for (i = 0; i < count; i++) {
+        if (!strcmp(infos[i].idstr, qemu_ram_get_idstr(block))) {
+            break;
+        }
+    }
+
+    if (i == count) {
+        return false;
+    }
+
+    if (infos[i].block_addr != qemu_ram_get_host_addr(block) ||
+        infos[i].block_pages !=
+            (qemu_ram_get_used_length(block) >> DIRTYRATE_PAGE_SIZE_SHIFT)) {
+        return false;
+    }
+
+    *matched = &infos[i];
+    return true;
+}
+
+static int compare_block_hash_info(struct block_dirty_info *info, int block_index)
+{
+    struct block_dirty_info *block_dinfo = NULL;
+    RAMBlock *block = NULL;
+
+    RAMBLOCK_FOREACH_MIGRATABLE(block) {
+        if (ram_block_skip(block) < 0) {
+            continue;
+        }
+        block_dinfo = NULL;
+        if (!find_block_matched(block, info, block_index + 1, &block_dinfo)) {
+            continue;
+        }
+        if (cal_block_dirty_rate(block_dinfo) < 0) {
+            return -1;
+        }
+        update_dirtyrate_stat(block_dinfo);
+    }
+    if (!dirty_stat.total_sample_count) {
+        return -1;
+    }
+
+    return 0;
+}
+
+
 static void calculate_dirtyrate(struct dirtyrate_config config, int64_t time)
 {
     /* todo */