Re: [PATCH v3 02/10] migration/dirtyrate: Add RamlockDirtyInfo to store sampled page info

[Dr. David Alan Gilbert]

* Chuan Zheng (zhengchuan@huawei.com) wrote:
> Add RamlockDirtyInfo to store sampled page info of each ramblock.
> 
> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> Signed-off-by: YanYing Zhuang <ann.zhuangyanying@huawei.com>
> ---
>  migration/dirtyrate.h | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
> index 914c363..9650566 100644
> --- a/migration/dirtyrate.h
> +++ b/migration/dirtyrate.h
> @@ -19,6 +19,11 @@
>   */
>  #define DIRTYRATE_DEFAULT_SAMPLE_PAGES            256
>  
> +/*
> + * Record ramblock idstr
> + */
> +#define RAMBLOCK_INFO_MAX_LEN                     256
> +
>  /* Take 1s as default for calculation duration */
>  #define DEFAULT_FETCH_DIRTYRATE_TIME_SEC          1
>  
> @@ -39,6 +44,19 @@ typedef enum {
>      CAL_DIRTY_RATE_END,
>  } CalculatingDirtyRateState;
>  
> +/*
> + * Store dirtypage info for each ramblock.
> + */
> +struct RamblockDirtyInfo {
> +    char idstr[RAMBLOCK_INFO_MAX_LEN]; /* idstr for each ramblock */

Can you remind me; why not just use RAMBlock* here of the block you're
interested in, rather than storing the name?

> +    uint8_t *ramblock_addr; /* base address of ramblock we measure */
> +    size_t ramblock_pages; /* sum of dividation by 4K pages for ramblock */

'dividation' is the wrong word, and 'sum' is only needed where you're
adding things together.  I think this is 'ramblock size in TARGET_PAGEs'

> +    size_t *sample_page_vfn; /* relative offset address for sampled page */
> +    unsigned int sample_pages_count; /* sum of sampled pages */
> +    unsigned int sample_dirty_count; /* sum of dirty pages we measure */

These are both 'count' rather than 'sum'

> +    uint8_t *hash_result; /* array of hash result for sampled pages */
> +};
> +
>  void *get_dirtyrate_thread(void *arg);
>  #endif
>  
> -- 
> 1.8.3.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK