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

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* 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