On Mon, Jan 24, 2022 at 10:10:37PM +0800, huang...@chinatelecom.cn wrote:
> diff --git a/cpus-common.c b/cpus-common.c
> index 6e73d3e..63159d6 100644
> --- a/cpus-common.c
> +++ b/cpus-common.c
> @@ -73,6 +73,7 @@ static int cpu_get_free_index(void)
> }
>
> CPUTailQ cpus = QTAILQ_HEAD_INITIALIZER(cpus);
> +unsigned int cpu_list_generation_id;
>
> void cpu_list_add(CPUState *cpu)
> {
> @@ -84,6 +85,7 @@ void cpu_list_add(CPUState *cpu)
> assert(!cpu_index_auto_assigned);
> }
> QTAILQ_INSERT_TAIL_RCU(&cpus, cpu, node);
> + cpu_list_generation_id++;
> }
>
> void cpu_list_remove(CPUState *cpu)
> @@ -96,6 +98,7 @@ void cpu_list_remove(CPUState *cpu)
>
> QTAILQ_REMOVE_RCU(&cpus, cpu, node);
> cpu->cpu_index = UNASSIGNED_CPU_INDEX;
> + cpu_list_generation_id++;
> }
Could you move the cpu list gen id changes into a separate patch?
>
> CPUState *qemu_get_cpu(int index)
> diff --git a/include/sysemu/dirtyrate.h b/include/sysemu/dirtyrate.h
> new file mode 100644
> index 0000000..ea4785f
> --- /dev/null
> +++ b/include/sysemu/dirtyrate.h
> @@ -0,0 +1,31 @@
> +/*
> + * dirty page rate helper functions
> + *
> + * Copyright (c) 2022 CHINA TELECOM CO.,LTD.
> + *
> + * Authors:
> + * Hyman Huang(黄勇) <huang...@chinatelecom.cn>
> + *
> + * 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_DIRTYRATE_H
> +#define QEMU_DIRTYRATE_H
> +
> +extern unsigned int cpu_list_generation_id;
How about exporting a function cpu_list_generation_id_get() from the cpu code,
rather than referencing it directly?
> +int64_t vcpu_calculate_dirtyrate(int64_t calc_time_ms,
> + int64_t init_time_ms,
> + VcpuStat *stat,
> + unsigned int flag,
> + bool one_shot)
> +{
> + DirtyPageRecord *records;
> + int64_t duration;
> + int64_t dirtyrate;
> + int i = 0;
> + unsigned int gen_id;
> +
> +retry:
> + cpu_list_lock();
> + gen_id = cpu_list_generation_id;
> + records = vcpu_dirty_stat_alloc(stat);
> + vcpu_dirty_stat_collect(stat, records, true);
> +
> + duration = dirty_stat_wait(calc_time_ms, init_time_ms);
> + cpu_list_unlock();
Should release the lock before sleep (dirty_stat_wait)?
> +
> + global_dirty_log_sync(flag, one_shot);
> +
> + cpu_list_lock();
> + if (gen_id != cpu_list_generation_id) {
> + g_free(records);
> + g_free(stat->rates);
> + cpu_list_unlock();
> + goto retry;
> + }
> + vcpu_dirty_stat_collect(stat, records, false);
> + cpu_list_unlock();
> +
> + for (i = 0; i < stat->nvcpu; i++) {
> + dirtyrate = do_calculate_dirtyrate(records[i], duration);
> +
> + stat->rates[i].id = i;
> + stat->rates[i].dirty_rate = dirtyrate;
> +
> + trace_dirtyrate_do_calculate_vcpu(i, dirtyrate);
> + }
> +
> + g_free(records);
> +
> + return duration;
> +}
Thanks,
--
Peter Xu
