* Chuan Zheng (zhengch...@huawei.com) wrote: > From: Zheng Chuan <zhengch...@huawei.com> > > Add dirtyrate statistics to record/update dirtyrate info. > > Signed-off-by: Zheng Chuan <zhengch...@huawei.com> > Signed-off-by: YanYing Zhang <ann.zhuangyany...@huawei.com> > --- > migration/dirtyrate.c | 47 ++++++++++++++++++++++++++++++----------------- > migration/dirtyrate.h | 11 +++++++++++ > 2 files changed, 41 insertions(+), 17 deletions(-) > > diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c > index fc652fb..6baf674 100644 > --- a/migration/dirtyrate.c > +++ b/migration/dirtyrate.c > @@ -13,19 +13,41 @@ > #include "dirtyrate.h" > > static uint64_t sample_pages_per_gigabytes = DIRTYRATE_DEFAULT_SAMPLE_PAGES; > -static uint64_t dirty_rate; /* MB/s */ > +static struct dirtyrate_statistics dirty_stat; > CalculatingDirtyRateStage calculating_dirty_rate_stage = CAL_DIRTY_RATE_INIT; > > -static bool calculate_dirtyrate(struct dirtyrate_config config, > - uint64_t *dirty_rate, int64_t time) > +static void reset_dirtyrate_stat(void) > { > - /* todo */ > - return true; > + dirty_stat.total_dirty_samples = 0; > + dirty_stat.total_sample_count = 0; > + dirty_stat.total_block_mem_MB = 0; > + dirty_stat.dirty_rate = 0; > +} > + > +static void update_dirtyrate_stat(struct block_dirty_info *info) > +{ > + dirty_stat.total_dirty_samples += info->sample_dirty_count; > + dirty_stat.total_sample_count += info->sample_pages_count; > + dirty_stat.total_block_mem_MB += (info->block_pages << > DIRTYRATE_PAGE_SIZE_SHIFT) >> PAGE_SIZE_SHIFT; > } > > -static void set_dirty_rate(uint64_t drate) > +static void update_dirtyrate(int64_t msec) > { > - dirty_rate = drate; > + uint64_t dirty_rate; > + unsigned int total_dirty_samples = dirty_stat.total_dirty_samples; > + unsigned int total_sample_count = dirty_stat.total_sample_count; > + unsigned long total_block_mem_MB = dirty_stat.total_block_mem_MB; > + > + dirty_rate = total_dirty_samples * total_block_mem_MB * > + 1000 / (total_sample_count * msec); > + > + dirty_stat.dirty_rate = dirty_rate; > +} > + > + > +static void calculate_dirtyrate(struct dirtyrate_config config, int64_t time) > +{ > + /* todo */ > } > > /* > @@ -42,21 +64,12 @@ static void > set_dirty_rate_stage(CalculatingDirtyRateStage ratestage) > void *get_dirtyrate_thread(void *arg) > { > struct dirtyrate_config config = *(struct dirtyrate_config *)arg; > - uint64_t dirty_rate; > - uint64_t hash_dirty_rate; > - bool query_succ; > int64_t msec = 0; > > set_dirty_rate_stage(CAL_DIRTY_RATE_ING); > > - query_succ = calculate_dirtyrate(config, &hash_dirty_rate, msec); > - if (!query_succ) { > - dirty_rate = 0; > - } else { > - dirty_rate = hash_dirty_rate; > - }
All this was only just added; it might be easier to create the update_dirtyrate function first. > + calculate_dirtyrate(config, msec); > > - set_dirty_rate(dirty_rate); > set_dirty_rate_stage(CAL_DIRTY_RATE_END); > > return NULL; > diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h > index 342b89f..2994535 100644 > --- a/migration/dirtyrate.h > +++ b/migration/dirtyrate.h > @@ -15,6 +15,9 @@ > > /* take 256 pages per GB for cal dirty rate */ > #define DIRTYRATE_DEFAULT_SAMPLE_PAGES 256 > +#define DIRTYRATE_PAGE_SIZE_SHIFT 12 > +#define BLOCK_INFO_MAX_LEN 256 > +#define PAGE_SIZE_SHIFT 20 I think you might also have used one of these #define's in a previous patch; so make sure the patches each compile in order. Also, can you please comment each one of these - I was confused by a lot of the calculations above because I don't quite understand what each of these is. I don't thinl 'BLOCK_INFO_MAX_LEN' is needed - becuase it's just a RAMBlock ID, and you can link to the RAMBlock. I'm not sure what DIRTYRATE_PAGE_SIZE_SHIFT is, or why it's different from TARGET_PAGE_BITS. > > struct dirtyrate_config { > uint64_t sample_pages_per_gigabytes; > @@ -33,6 +36,14 @@ typedef enum { > CAL_DIRTY_RATE_END = 2, > } CalculatingDirtyRateStage; > > +struct dirtyrate_statistics { > + unsigned int total_dirty_samples; > + unsigned int total_sample_count; > + unsigned long total_block_mem_MB; 'long' is normally a bad idea - we use it in a few places and it was generally a bad idea; size_t for a size is much better. > + int64_t dirty_rate; Is this blocks/sec, MB/s what - please comment it. Dave > +}; > + > + > /* > * Store dirtypage info for each block. > */ > -- > 1.8.3.1 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK