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

[Zheng Chuan]

On 2020/8/7 0:59, Dr. David Alan Gilbert wrote:
> * Zheng Chuan (zhengchuan@huawei.com) wrote:
>>
>>
>> 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.
> 
> Right yes; but that 'hash_result' is an array of hash values, one
> for each of the pages that you did measure?
> 
> Dave
> 
Yes, hash_result stores the results from qcrypto_hash_bytesv() for each of the 
4K pages i sampled.

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