Re: [PATCH v5 03/12] migration/dirtyrate: Add RamlockDirtyInfo to store sampled page info

[Zheng Chuan]

On 2020/8/26 17:29, David Edmondson wrote:
> On Monday, 2020-08-24 at 17:14:31 +08, Chuan Zheng wrote:
> 
>> Add RamlockDirtyInfo to store sampled page info of each ramblock.
>>
>> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
>> ---
>>  migration/dirtyrate.h | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>>
>> diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
>> index 33669b7..70000da 100644
>> --- a/migration/dirtyrate.h
>> +++ b/migration/dirtyrate.h
>> @@ -19,6 +19,11 @@
>>   */
>>  #define DIRTYRATE_DEFAULT_SAMPLE_PAGES            512
>>  
>> +/*
>> + * Record ramblock idstr
>> + */
>> +#define RAMBLOCK_INFO_MAX_LEN                     256
>> +
>>  /* Take 1s as default for calculation duration */
>>  #define DEFAULT_FETCH_DIRTYRATE_TIME_SEC          1
>>  
>> @@ -27,6 +32,19 @@ struct DirtyRateConfig {
>>      int64_t sample_period_seconds; /* time duration between two sampling */
>>  };
>>  
>> +/*
>> + * Store dirtypage info for each ramblock.
>> + */
>> +struct RamblockDirtyInfo {
>> +    char idstr[RAMBLOCK_INFO_MAX_LEN]; /* idstr for each ramblock */
>> +    uint8_t *ramblock_addr; /* base address of ramblock we measure */
>> +    uint64_t ramblock_pages; /* ramblock size in 4K-page */
> 
> It's probably a stupid question, but why not store a pointer to the
> RAMBlock rather than copying some of the details?
> 
>> +    uint64_t *sample_page_vfn; /* relative offset address for sampled page 
>> */
>> +    uint64_t sample_pages_count; /* count of sampled pages */
>> +    uint64_t sample_dirty_count; /* cout of dirty pages we measure */
> 
> "cout" -> "count"
> 
Hi, David.
Thank you for review.
Actually I have resent [PATCH V5], but it's my fault to forget to add resent 
tag which may lead to confusing.
In new patch series, this spell mistake is fixed.
Please refer to the newer patch series:)

>> +    uint32_t *hash_result; /* array of hash result for sampled pages */
>> +};
>> +
>>  void *get_dirtyrate_thread(void *arg);
>>  #endif
>>  
>> -- 
>> 1.8.3.1
> 
> dme.
>