[v1] *** A Method for evaluating dirty page rate ***

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

Posted by Chuan Zheng 5 years, 3 months ago

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 {
+    char idstr[BLOCK_INFO_MAX_LEN];
+    uint8_t *block_addr;
+    unsigned long block_pages;
+    unsigned long *sample_page_vfn;
+    unsigned int sample_pages_count;
+    unsigned int sample_dirty_count;
+    uint8_t *hash_result;
+};
+
 void *get_dirtyrate_thread(void *arg);
 #endif
 
-- 
1.8.3.1

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

Posted by Dr. David Alan Gilbert 5 years, 3 months ago

* 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.

> +    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

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

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

Posted by Zheng Chuan 5 years, 3 months ago


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
> 
> 
> .
>

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

Posted by Dr. David Alan Gilbert 5 years, 3 months ago

* 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

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

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

Posted by Zheng Chuan 5 years, 3 months ago


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
>>>
>>>
>>> .
>>>
>>