[v1] *** A Method for evaluating dirty page rate ***

[RFC PATCH 1/8] migration/dirtyrate: Add get_dirtyrate_thread() function

Posted by Chuan Zheng 5 years, 3 months ago

From: Zheng Chuan <zhengchuan@huawei.com>

Add get_dirtyrate_thread() functions

Signed-off-by: Zheng Chuan <zhengchuan@huawei.com>
Signed-off-by: YanYing Zhang <ann.zhuangyanying@huawei.com>
---
 migration/dirtyrate.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++
 migration/dirtyrate.h | 38 +++++++++++++++++++++++++++++++
 2 files changed, 101 insertions(+)
 create mode 100644 migration/dirtyrate.c
 create mode 100644 migration/dirtyrate.h

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
new file mode 100644
index 0000000..fc652fb
--- /dev/null
+++ b/migration/dirtyrate.c
@@ -0,0 +1,63 @@
+/*
+ * Dirtyrate implement code
+ *
+ * Copyright (c) 2017-2020 HUAWEI TECHNOLOGIES CO.,LTD.
+ *
+ * Authors:
+ *  Chuan Zheng <zhengchuan@huawei.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "dirtyrate.h"
+
+static uint64_t sample_pages_per_gigabytes = DIRTYRATE_DEFAULT_SAMPLE_PAGES;
+static uint64_t dirty_rate; /* MB/s */
+CalculatingDirtyRateStage calculating_dirty_rate_stage = CAL_DIRTY_RATE_INIT;
+
+static bool calculate_dirtyrate(struct dirtyrate_config config,
+                        uint64_t *dirty_rate, int64_t time)
+{
+    /* todo */
+    return true;
+}
+
+static void set_dirty_rate(uint64_t drate)
+{
+    dirty_rate = drate;
+}
+
+/*
+ * There are multithread will write/read *calculating_dirty_rate_stage*,
+ * we can protect only one thread write/read it by libvirt api.
+ * So we don't add mutex_lock to protect it here, but we must calculate
+ * dirty_rate by libvirt api.
+ */
+static void set_dirty_rate_stage(CalculatingDirtyRateStage ratestage)
+{
+    calculating_dirty_rate_stage = ratestage;
+}
+
+void *get_dirtyrate_thread(void *arg)
+{
+    struct dirtyrate_config config = *(struct dirtyrate_config *)arg;
+    uint64_t dirty_rate;
+    uint64_t hash_dirty_rate;
+    bool query_succ;
+    int64_t msec = 0;
+ 
+    set_dirty_rate_stage(CAL_DIRTY_RATE_ING);
+
+    query_succ = calculate_dirtyrate(config, &hash_dirty_rate, msec);
+    if (!query_succ) {
+        dirty_rate = 0;
+    } else {
+        dirty_rate = hash_dirty_rate;
+    }
+
+    set_dirty_rate(dirty_rate);
+    set_dirty_rate_stage(CAL_DIRTY_RATE_END);
+
+    return NULL;
+}
diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
new file mode 100644
index 0000000..9a5c228
--- /dev/null
+++ b/migration/dirtyrate.h
@@ -0,0 +1,38 @@
+/*
+ *  Dirtyrate common functions
+ *
+ *  Copyright (c) 2020 HUAWEI TECHNOLOGIES CO., LTD.
+ *
+ *  Authors:
+ *  Chuan Zheng <zhengchuan@huawei.com>
+ *
+ *  This work is licensed under the terms of the GNU GPL, version 2 or later.
+ *  See the COPYING file in the top-level directory.
+ */
+
+#ifndef QEMU_MIGRATION_DIRTYRATE_H
+#define QEMU_MIGRATION_DIRTYRATE_H
+
+/* take 256 pages per GB for cal dirty rate */
+#define DIRTYRATE_DEFAULT_SAMPLE_PAGES    256
+
+struct dirtyrate_config {
+    uint64_t sample_pages_per_gigabytes;
+    int64_t sample_period_seconds;
+};
+
+/*
+ *  To record calculate dirty_rate status:
+ *  0: initial status, calculating thread is not be created here.
+ *  1: calculating thread is created.
+ *  2: calculating thread is end, we can get result.
+ */
+typedef enum {
+    CAL_DIRTY_RATE_INIT  = 0,
+    CAL_DIRTY_RATE_ING   = 1,
+    CAL_DIRTY_RATE_END   = 2,
+} CalculatingDirtyRateStage;
+
+void *get_dirtyrate_thread(void *arg);
+#endif
+
-- 
1.8.3.1

Re: [RFC PATCH 1/8] migration/dirtyrate: Add get_dirtyrate_thread() function

Posted by Dr. David Alan Gilbert 5 years, 3 months ago

* Chuan Zheng (zhengchuan@huawei.com) wrote:
> From: Zheng Chuan <zhengchuan@huawei.com>
> 
> Add get_dirtyrate_thread() functions
> 
> Signed-off-by: Zheng Chuan <zhengchuan@huawei.com>
> Signed-off-by: YanYing Zhang <ann.zhuangyanying@huawei.com>
> ---
>  migration/dirtyrate.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  migration/dirtyrate.h | 38 +++++++++++++++++++++++++++++++
>  2 files changed, 101 insertions(+)
>  create mode 100644 migration/dirtyrate.c
>  create mode 100644 migration/dirtyrate.h
> 
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> new file mode 100644
> index 0000000..fc652fb
> --- /dev/null
> +++ b/migration/dirtyrate.c
> @@ -0,0 +1,63 @@
> +/*
> + * Dirtyrate implement code
> + *
> + * Copyright (c) 2017-2020 HUAWEI TECHNOLOGIES CO.,LTD.
> + *
> + * Authors:
> + *  Chuan Zheng <zhengchuan@huawei.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "dirtyrate.h"
> +
> +static uint64_t sample_pages_per_gigabytes = DIRTYRATE_DEFAULT_SAMPLE_PAGES;
> +static uint64_t dirty_rate; /* MB/s */
> +CalculatingDirtyRateStage calculating_dirty_rate_stage = CAL_DIRTY_RATE_INIT;
> +
> +static bool calculate_dirtyrate(struct dirtyrate_config config,
> +                        uint64_t *dirty_rate, int64_t time)
> +{
> +    /* todo */
> +    return true;

It would be better to make this return false until you fill it in!

> +}
> +
> +static void set_dirty_rate(uint64_t drate)
> +{
> +    dirty_rate = drate;
> +}
> +
> +/*
> + * There are multithread will write/read *calculating_dirty_rate_stage*,
> + * we can protect only one thread write/read it by libvirt api.
> + * So we don't add mutex_lock to protect it here, but we must calculate
> + * dirty_rate by libvirt api.
> + */
> +static void set_dirty_rate_stage(CalculatingDirtyRateStage ratestage)
> +{
> +    calculating_dirty_rate_stage = ratestage;
> +}

I don't think I understand the threading comment here; when you say the
'libvirt api' do youmean QMP?  Maybe you could do this with an
atomic_cmpxchg like we do in migrate_set_state?

> +
> +void *get_dirtyrate_thread(void *arg)
> +{
> +    struct dirtyrate_config config = *(struct dirtyrate_config *)arg;
> +    uint64_t dirty_rate;
> +    uint64_t hash_dirty_rate;
> +    bool query_succ;
> +    int64_t msec = 0;
> + 
> +    set_dirty_rate_stage(CAL_DIRTY_RATE_ING);
> +
> +    query_succ = calculate_dirtyrate(config, &hash_dirty_rate, msec);
> +    if (!query_succ) {
> +        dirty_rate = 0;
> +    } else {
> +        dirty_rate = hash_dirty_rate;
> +    }
> +
> +    set_dirty_rate(dirty_rate);
> +    set_dirty_rate_stage(CAL_DIRTY_RATE_END);
> +
> +    return NULL;
> +}
> diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
> new file mode 100644
> index 0000000..9a5c228
> --- /dev/null
> +++ b/migration/dirtyrate.h
> @@ -0,0 +1,38 @@
> +/*
> + *  Dirtyrate common functions
> + *
> + *  Copyright (c) 2020 HUAWEI TECHNOLOGIES CO., LTD.
> + *
> + *  Authors:
> + *  Chuan Zheng <zhengchuan@huawei.com>
> + *
> + *  This work is licensed under the terms of the GNU GPL, version 2 or later.
> + *  See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef QEMU_MIGRATION_DIRTYRATE_H
> +#define QEMU_MIGRATION_DIRTYRATE_H
> +
> +/* take 256 pages per GB for cal dirty rate */
> +#define DIRTYRATE_DEFAULT_SAMPLE_PAGES    256
> +
> +struct dirtyrate_config {
> +    uint64_t sample_pages_per_gigabytes;
> +    int64_t sample_period_seconds;
> +};
> +
> +/*
> + *  To record calculate dirty_rate status:
> + *  0: initial status, calculating thread is not be created here.
> + *  1: calculating thread is created.
> + *  2: calculating thread is end, we can get result.
> + */
> +typedef enum {
> +    CAL_DIRTY_RATE_INIT  = 0,
> +    CAL_DIRTY_RATE_ING   = 1,

I'm not sure why ING?


> +    CAL_DIRTY_RATE_END   = 2,
> +} CalculatingDirtyRateStage;
> +
> +void *get_dirtyrate_thread(void *arg);
> +#endif
> +
> -- 
> 1.8.3.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [RFC PATCH 1/8] migration/dirtyrate: Add get_dirtyrate_thread() function

Posted by Zheng Chuan 5 years, 3 months ago


On 2020/8/5 0:23, Dr. David Alan Gilbert wrote:
> * Chuan Zheng (zhengchuan@huawei.com) wrote:
>> From: Zheng Chuan <zhengchuan@huawei.com>
>>
>> Add get_dirtyrate_thread() functions
>>
>> Signed-off-by: Zheng Chuan <zhengchuan@huawei.com>
>> Signed-off-by: YanYing Zhang <ann.zhuangyanying@huawei.com>
>> ---
>>  migration/dirtyrate.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  migration/dirtyrate.h | 38 +++++++++++++++++++++++++++++++
>>  2 files changed, 101 insertions(+)
>>  create mode 100644 migration/dirtyrate.c
>>  create mode 100644 migration/dirtyrate.h
>>
>> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
>> new file mode 100644
>> index 0000000..fc652fb
>> --- /dev/null
>> +++ b/migration/dirtyrate.c
>> @@ -0,0 +1,63 @@
>> +/*
>> + * Dirtyrate implement code
>> + *
>> + * Copyright (c) 2017-2020 HUAWEI TECHNOLOGIES CO.,LTD.
>> + *
>> + * Authors:
>> + *  Chuan Zheng <zhengchuan@huawei.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + */
>> +
>> +#include "dirtyrate.h"
>> +
>> +static uint64_t sample_pages_per_gigabytes = DIRTYRATE_DEFAULT_SAMPLE_PAGES;
>> +static uint64_t dirty_rate; /* MB/s */
>> +CalculatingDirtyRateStage calculating_dirty_rate_stage = CAL_DIRTY_RATE_INIT;
>> +
>> +static bool calculate_dirtyrate(struct dirtyrate_config config,
>> +                        uint64_t *dirty_rate, int64_t time)
>> +{
>> +    /* todo */
>> +    return true;
> 
> It would be better to make this return false until you fill it in!
> 
Sure, I'll fix that in V2.
>> +}
>> +
>> +static void set_dirty_rate(uint64_t drate)
>> +{
>> +    dirty_rate = drate;
>> +}
>> +
>> +/*
>> + * There are multithread will write/read *calculating_dirty_rate_stage*,
>> + * we can protect only one thread write/read it by libvirt api.
>> + * So we don't add mutex_lock to protect it here, but we must calculate
>> + * dirty_rate by libvirt api.
>> + */
>> +static void set_dirty_rate_stage(CalculatingDirtyRateStage ratestage)
>> +{
>> +    calculating_dirty_rate_stage = ratestage;
>> +}
> 
> I don't think I understand the threading comment here; when you say the
> 'libvirt api' do youmean QMP?  Maybe you could do this with an
> atomic_cmpxchg like we do in migrate_set_state?
> 
Yes, I mean QMP, atomic_cmpxchg should be better:)
I'll fix that and make comments more clear in V2.

>> +
>> +void *get_dirtyrate_thread(void *arg)
>> +{
>> +    struct dirtyrate_config config = *(struct dirtyrate_config *)arg;
>> +    uint64_t dirty_rate;
>> +    uint64_t hash_dirty_rate;
>> +    bool query_succ;
>> +    int64_t msec = 0;
>> + 
>> +    set_dirty_rate_stage(CAL_DIRTY_RATE_ING);
>> +
>> +    query_succ = calculate_dirtyrate(config, &hash_dirty_rate, msec);
>> +    if (!query_succ) {
>> +        dirty_rate = 0;
>> +    } else {
>> +        dirty_rate = hash_dirty_rate;
>> +    }
>> +
>> +    set_dirty_rate(dirty_rate);
>> +    set_dirty_rate_stage(CAL_DIRTY_RATE_END);
>> +
>> +    return NULL;
>> +}
>> diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
>> new file mode 100644
>> index 0000000..9a5c228
>> --- /dev/null
>> +++ b/migration/dirtyrate.h
>> @@ -0,0 +1,38 @@
>> +/*
>> + *  Dirtyrate common functions
>> + *
>> + *  Copyright (c) 2020 HUAWEI TECHNOLOGIES CO., LTD.
>> + *
>> + *  Authors:
>> + *  Chuan Zheng <zhengchuan@huawei.com>
>> + *
>> + *  This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + *  See the COPYING file in the top-level directory.
>> + */
>> +
>> +#ifndef QEMU_MIGRATION_DIRTYRATE_H
>> +#define QEMU_MIGRATION_DIRTYRATE_H
>> +
>> +/* take 256 pages per GB for cal dirty rate */
>> +#define DIRTYRATE_DEFAULT_SAMPLE_PAGES    256
>> +
>> +struct dirtyrate_config {
>> +    uint64_t sample_pages_per_gigabytes;
>> +    int64_t sample_period_seconds;
>> +};
>> +
>> +/*
>> + *  To record calculate dirty_rate status:
>> + *  0: initial status, calculating thread is not be created here.
>> + *  1: calculating thread is created.
>> + *  2: calculating thread is end, we can get result.
>> + */
>> +typedef enum {
>> +    CAL_DIRTY_RATE_INIT  = 0,
>> +    CAL_DIRTY_RATE_ING   = 1,
> 
> I'm not sure why ING?
Maybe ACTIVE is better, i'll fix that.
> 
> 
>> +    CAL_DIRTY_RATE_END   = 2,
>> +} CalculatingDirtyRateStage;
>> +
>> +void *get_dirtyrate_thread(void *arg);
>> +#endif
>> +
>> -- 
>> 1.8.3.1
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 
> 
> .
>