Re: [RFC PATCH 2/8] migration/dirtyrate: Add block_dirty_info to store dirtypage info

[Zheng Chuan <zhengchuan@huawei.com>]

On 2020/8/5 0:28, Dr. David Alan Gilbert wrote:
> * Chuan Zheng (zhengchuan@huawei.com) wrote:
>> From: Zheng Chuan <zhengchuan@huawei.com>
>>
>> Add block_dirty_info to store dirtypage info for each ramblock
>>
>> Signed-off-by: Zheng Chuan <zhengchuan@huawei.com>
>> Signed-off-by: YanYing Zhang <ann.zhuangyanying@huawei.com>
>> ---
>>  migration/dirtyrate.h | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
>> index 9a5c228..342b89f 100644
>> --- a/migration/dirtyrate.h
>> +++ b/migration/dirtyrate.h
>> @@ -33,6 +33,19 @@ typedef enum {
>>      CAL_DIRTY_RATE_END   = 2,
>>  } CalculatingDirtyRateStage;
>>  
>> +/* 
>> + * Store dirtypage info for each block.
>> + */
>> +struct block_dirty_info {
> 
> Please call this ramblock_dirty_info; we use 'block' a lot to mean
> disk block and it gets confusing.
> 
Sure, ramblock_dirty_info is better.

>> +    char idstr[BLOCK_INFO_MAX_LEN];
> 
> Is there a reason you don't just use a RAMBlock *  here?
> 
>> +    uint8_t *block_addr;
>> +    unsigned long block_pages;
>> +    unsigned long *sample_page_vfn;
> 
> Please comment these; if I understand correctly, that's an array
> of page indexes into the block generated from the random numbers
> 
>> +    unsigned int sample_pages_count;
>> +    unsigned int sample_dirty_count;
>> +    uint8_t *hash_result;
> 
> If I understand, this is an array of hashes end-to-end for
> all the pages in this RAMBlock?
> 
> Dave
> 
Actually, we do not go through all pages of the RAMBlock but sample
some pages （for example, 256 pages per Gigabit）to make it faster.
Obviously it will sacrifice accuracy, but it still looks good enough
under practical test.

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