* [Qemu-devel] [PATCH 0/2] migration/postcopy: simplify postcopy_chunk_hostpages_pass @ 2019-08-06 0:46 Wei Yang 2019-08-06 0:46 ` [Qemu-devel] [PATCH 1/2] migration/postcopy: simplify calculation of run_start and fixup_start_addr Wei Yang ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Wei Yang @ 2019-08-06 0:46 UTC (permalink / raw) To: qemu-devel; +Cc: Wei Yang, dgilbert, quintela When looking into function postcopy_chunk_hostpages_pass(), we could use alignment calculation to simplify it. Wei Yang (2): migration/postcopy: simplify calculation of run_start and fixup_start_addr migration/postcopy: use QEMU_IS_ALIGNED to replace host_offset migration/ram.c | 37 +++++++------------------------------ 1 file changed, 7 insertions(+), 30 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH 1/2] migration/postcopy: simplify calculation of run_start and fixup_start_addr 2019-08-06 0:46 [Qemu-devel] [PATCH 0/2] migration/postcopy: simplify postcopy_chunk_hostpages_pass Wei Yang @ 2019-08-06 0:46 ` Wei Yang 2019-08-06 14:51 ` Dr. David Alan Gilbert 2019-08-06 0:46 ` [Qemu-devel] [PATCH 2/2] migration/postcopy: use QEMU_IS_ALIGNED to replace host_offset Wei Yang 2019-08-07 18:43 ` [Qemu-devel] [PATCH 0/2] migration/postcopy: simplify postcopy_chunk_hostpages_pass Dr. David Alan Gilbert 2 siblings, 1 reply; 7+ messages in thread From: Wei Yang @ 2019-08-06 0:46 UTC (permalink / raw) To: qemu-devel; +Cc: Wei Yang, dgilbert, quintela The purpose of the calculation is to find a HostPage which is partially dirty. * fixup_start_addr points to the start of the HostPage to discard * run_start points to the next HostPage to check While in the middle stage, there would two cases for run_start: * aligned with HostPage means this is not partially dirty * not aligned means this is partially dirty When it is aligned, no work and calculation is necessary. run_start already points to the start of next HostPage and is ready to continue. When it is not aligned, the calculation could be simplified with: * fixup_start_addr = QEMU_ALIGN_DOWN(run_start, host_ratio) * run_start = QEMU_ALIGN_UP(run_start, host_ratio) By doing so, run_start always points to the next HostPage to check. fixup_start_addr always points to the HostPage to discard. Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> --- migration/ram.c | 34 +++++++--------------------------- 1 file changed, 7 insertions(+), 27 deletions(-) diff --git a/migration/ram.c b/migration/ram.c index c9585487ac..d86661a015 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -2956,7 +2956,6 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, bool unsent_pass, } while (run_start < pages) { - unsigned long fixup_start_addr; unsigned long host_offset; /* @@ -2964,45 +2963,26 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, bool unsent_pass, * page, then we need to fixup this host page. */ host_offset = run_start % host_ratio; - if (host_offset) { - fixup_start_addr = run_start - host_offset; - /* - * This host page has gone, the next loop iteration starts - * from after the fixup - */ - run_start = fixup_start_addr + host_ratio; - } else { + if (!host_offset) { /* Find the end of this run */ - unsigned long run_end; if (unsent_pass) { - run_end = find_next_bit(unsentmap, pages, run_start + 1); + run_start = find_next_bit(unsentmap, pages, run_start + 1); } else { - run_end = find_next_zero_bit(bitmap, pages, run_start + 1); + run_start = find_next_zero_bit(bitmap, pages, run_start + 1); } /* * If the end isn't at the start of a host page, then the * run doesn't finish at the end of a host page * and we need to discard. */ - host_offset = run_end % host_ratio; - if (host_offset) { - fixup_start_addr = run_end - host_offset; - /* - * This host page has gone, the next loop iteration starts - * from after the fixup - */ - run_start = fixup_start_addr + host_ratio; - } else { - /* - * No discards on this iteration, next loop starts from - * next sent/dirty page - */ - run_start = run_end + 1; - } + host_offset = run_start % host_ratio; } if (host_offset) { unsigned long page; + unsigned long fixup_start_addr = QEMU_ALIGN_DOWN(run_start, + host_ratio); + run_start = QEMU_ALIGN_UP(run_start, host_ratio); /* Tell the destination to discard this page */ if (unsent_pass || !test_bit(fixup_start_addr, unsentmap)) { -- 2.17.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] migration/postcopy: simplify calculation of run_start and fixup_start_addr 2019-08-06 0:46 ` [Qemu-devel] [PATCH 1/2] migration/postcopy: simplify calculation of run_start and fixup_start_addr Wei Yang @ 2019-08-06 14:51 ` Dr. David Alan Gilbert 2019-08-07 0:14 ` Wei Yang 0 siblings, 1 reply; 7+ messages in thread From: Dr. David Alan Gilbert @ 2019-08-06 14:51 UTC (permalink / raw) To: Wei Yang; +Cc: qemu-devel, quintela * Wei Yang (richardw.yang@linux.intel.com) wrote: > The purpose of the calculation is to find a HostPage which is partially > dirty. > > * fixup_start_addr points to the start of the HostPage to discard > * run_start points to the next HostPage to check > > While in the middle stage, there would two cases for run_start: > > * aligned with HostPage means this is not partially dirty > * not aligned means this is partially dirty > > When it is aligned, no work and calculation is necessary. run_start > already points to the start of next HostPage and is ready to continue. > > When it is not aligned, the calculation could be simplified with: > > * fixup_start_addr = QEMU_ALIGN_DOWN(run_start, host_ratio) > * run_start = QEMU_ALIGN_UP(run_start, host_ratio) > > By doing so, run_start always points to the next HostPage to check. > fixup_start_addr always points to the HostPage to discard. > > Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> Yes that's a lot shorter; note it doess it on top of your cleanup from a couple of weeks back. Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > migration/ram.c | 34 +++++++--------------------------- > 1 file changed, 7 insertions(+), 27 deletions(-) > > diff --git a/migration/ram.c b/migration/ram.c > index c9585487ac..d86661a015 100644 > --- a/migration/ram.c > +++ b/migration/ram.c > @@ -2956,7 +2956,6 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, bool unsent_pass, > } > > while (run_start < pages) { > - unsigned long fixup_start_addr; > unsigned long host_offset; > > /* > @@ -2964,45 +2963,26 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, bool unsent_pass, > * page, then we need to fixup this host page. > */ > host_offset = run_start % host_ratio; > - if (host_offset) { > - fixup_start_addr = run_start - host_offset; > - /* > - * This host page has gone, the next loop iteration starts > - * from after the fixup > - */ > - run_start = fixup_start_addr + host_ratio; > - } else { > + if (!host_offset) { > /* Find the end of this run */ > - unsigned long run_end; > if (unsent_pass) { > - run_end = find_next_bit(unsentmap, pages, run_start + 1); > + run_start = find_next_bit(unsentmap, pages, run_start + 1); > } else { > - run_end = find_next_zero_bit(bitmap, pages, run_start + 1); > + run_start = find_next_zero_bit(bitmap, pages, run_start + 1); > } > /* > * If the end isn't at the start of a host page, then the > * run doesn't finish at the end of a host page > * and we need to discard. > */ > - host_offset = run_end % host_ratio; > - if (host_offset) { > - fixup_start_addr = run_end - host_offset; > - /* > - * This host page has gone, the next loop iteration starts > - * from after the fixup > - */ > - run_start = fixup_start_addr + host_ratio; > - } else { > - /* > - * No discards on this iteration, next loop starts from > - * next sent/dirty page > - */ > - run_start = run_end + 1; > - } > + host_offset = run_start % host_ratio; > } > > if (host_offset) { > unsigned long page; > + unsigned long fixup_start_addr = QEMU_ALIGN_DOWN(run_start, > + host_ratio); > + run_start = QEMU_ALIGN_UP(run_start, host_ratio); > > /* Tell the destination to discard this page */ > if (unsent_pass || !test_bit(fixup_start_addr, unsentmap)) { > -- > 2.17.1 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] migration/postcopy: simplify calculation of run_start and fixup_start_addr 2019-08-06 14:51 ` Dr. David Alan Gilbert @ 2019-08-07 0:14 ` Wei Yang 0 siblings, 0 replies; 7+ messages in thread From: Wei Yang @ 2019-08-07 0:14 UTC (permalink / raw) To: Dr. David Alan Gilbert; +Cc: quintela, Wei Yang, qemu-devel On Tue, Aug 06, 2019 at 03:51:17PM +0100, Dr. David Alan Gilbert wrote: >* Wei Yang (richardw.yang@linux.intel.com) wrote: >> The purpose of the calculation is to find a HostPage which is partially >> dirty. >> >> * fixup_start_addr points to the start of the HostPage to discard >> * run_start points to the next HostPage to check >> >> While in the middle stage, there would two cases for run_start: >> >> * aligned with HostPage means this is not partially dirty >> * not aligned means this is partially dirty >> >> When it is aligned, no work and calculation is necessary. run_start >> already points to the start of next HostPage and is ready to continue. >> >> When it is not aligned, the calculation could be simplified with: >> >> * fixup_start_addr = QEMU_ALIGN_DOWN(run_start, host_ratio) >> * run_start = QEMU_ALIGN_UP(run_start, host_ratio) >> >> By doing so, run_start always points to the next HostPage to check. >> fixup_start_addr always points to the HostPage to discard. >> >> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> > >Yes that's a lot shorter; note it doess it on top of your cleanup >from a couple of weeks back. > You are right, I forget to mention it :-) >Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > >> --- >> migration/ram.c | 34 +++++++--------------------------- >> 1 file changed, 7 insertions(+), 27 deletions(-) >> >> diff --git a/migration/ram.c b/migration/ram.c >> index c9585487ac..d86661a015 100644 >> --- a/migration/ram.c >> +++ b/migration/ram.c >> @@ -2956,7 +2956,6 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, bool unsent_pass, >> } >> >> while (run_start < pages) { >> - unsigned long fixup_start_addr; >> unsigned long host_offset; >> >> /* >> @@ -2964,45 +2963,26 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, bool unsent_pass, >> * page, then we need to fixup this host page. >> */ >> host_offset = run_start % host_ratio; >> - if (host_offset) { >> - fixup_start_addr = run_start - host_offset; >> - /* >> - * This host page has gone, the next loop iteration starts >> - * from after the fixup >> - */ >> - run_start = fixup_start_addr + host_ratio; >> - } else { >> + if (!host_offset) { >> /* Find the end of this run */ >> - unsigned long run_end; >> if (unsent_pass) { >> - run_end = find_next_bit(unsentmap, pages, run_start + 1); >> + run_start = find_next_bit(unsentmap, pages, run_start + 1); >> } else { >> - run_end = find_next_zero_bit(bitmap, pages, run_start + 1); >> + run_start = find_next_zero_bit(bitmap, pages, run_start + 1); >> } >> /* >> * If the end isn't at the start of a host page, then the >> * run doesn't finish at the end of a host page >> * and we need to discard. >> */ >> - host_offset = run_end % host_ratio; >> - if (host_offset) { >> - fixup_start_addr = run_end - host_offset; >> - /* >> - * This host page has gone, the next loop iteration starts >> - * from after the fixup >> - */ >> - run_start = fixup_start_addr + host_ratio; >> - } else { >> - /* >> - * No discards on this iteration, next loop starts from >> - * next sent/dirty page >> - */ >> - run_start = run_end + 1; >> - } >> + host_offset = run_start % host_ratio; >> } >> >> if (host_offset) { >> unsigned long page; >> + unsigned long fixup_start_addr = QEMU_ALIGN_DOWN(run_start, >> + host_ratio); >> + run_start = QEMU_ALIGN_UP(run_start, host_ratio); >> >> /* Tell the destination to discard this page */ >> if (unsent_pass || !test_bit(fixup_start_addr, unsentmap)) { >> -- >> 2.17.1 >> >-- >Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK -- Wei Yang Help you, Help me ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH 2/2] migration/postcopy: use QEMU_IS_ALIGNED to replace host_offset 2019-08-06 0:46 [Qemu-devel] [PATCH 0/2] migration/postcopy: simplify postcopy_chunk_hostpages_pass Wei Yang 2019-08-06 0:46 ` [Qemu-devel] [PATCH 1/2] migration/postcopy: simplify calculation of run_start and fixup_start_addr Wei Yang @ 2019-08-06 0:46 ` Wei Yang 2019-08-06 15:47 ` Dr. David Alan Gilbert 2019-08-07 18:43 ` [Qemu-devel] [PATCH 0/2] migration/postcopy: simplify postcopy_chunk_hostpages_pass Dr. David Alan Gilbert 2 siblings, 1 reply; 7+ messages in thread From: Wei Yang @ 2019-08-06 0:46 UTC (permalink / raw) To: qemu-devel; +Cc: Wei Yang, dgilbert, quintela Use QEMU_IS_ALIGNED for the check, it would be more consistent with other align calculations. Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> --- migration/ram.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/migration/ram.c b/migration/ram.c index d86661a015..dfbf71c580 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -2956,14 +2956,12 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, bool unsent_pass, } while (run_start < pages) { - unsigned long host_offset; /* * If the start of this run of pages is in the middle of a host * page, then we need to fixup this host page. */ - host_offset = run_start % host_ratio; - if (!host_offset) { + if (QEMU_IS_ALIGNED(run_start, host_ratio)) { /* Find the end of this run */ if (unsent_pass) { run_start = find_next_bit(unsentmap, pages, run_start + 1); @@ -2975,10 +2973,9 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, bool unsent_pass, * run doesn't finish at the end of a host page * and we need to discard. */ - host_offset = run_start % host_ratio; } - if (host_offset) { + if (!QEMU_IS_ALIGNED(run_start, host_ratio)) { unsigned long page; unsigned long fixup_start_addr = QEMU_ALIGN_DOWN(run_start, host_ratio); -- 2.17.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] migration/postcopy: use QEMU_IS_ALIGNED to replace host_offset 2019-08-06 0:46 ` [Qemu-devel] [PATCH 2/2] migration/postcopy: use QEMU_IS_ALIGNED to replace host_offset Wei Yang @ 2019-08-06 15:47 ` Dr. David Alan Gilbert 0 siblings, 0 replies; 7+ messages in thread From: Dr. David Alan Gilbert @ 2019-08-06 15:47 UTC (permalink / raw) To: Wei Yang; +Cc: qemu-devel, quintela * Wei Yang (richardw.yang@linux.intel.com) wrote: > Use QEMU_IS_ALIGNED for the check, it would be more consistent with > other align calculations. > > Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > migration/ram.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/migration/ram.c b/migration/ram.c > index d86661a015..dfbf71c580 100644 > --- a/migration/ram.c > +++ b/migration/ram.c > @@ -2956,14 +2956,12 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, bool unsent_pass, > } > > while (run_start < pages) { > - unsigned long host_offset; > > /* > * If the start of this run of pages is in the middle of a host > * page, then we need to fixup this host page. > */ > - host_offset = run_start % host_ratio; > - if (!host_offset) { > + if (QEMU_IS_ALIGNED(run_start, host_ratio)) { > /* Find the end of this run */ > if (unsent_pass) { > run_start = find_next_bit(unsentmap, pages, run_start + 1); > @@ -2975,10 +2973,9 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, bool unsent_pass, > * run doesn't finish at the end of a host page > * and we need to discard. > */ > - host_offset = run_start % host_ratio; > } > > - if (host_offset) { > + if (!QEMU_IS_ALIGNED(run_start, host_ratio)) { > unsigned long page; > unsigned long fixup_start_addr = QEMU_ALIGN_DOWN(run_start, > host_ratio); > -- > 2.17.1 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] migration/postcopy: simplify postcopy_chunk_hostpages_pass 2019-08-06 0:46 [Qemu-devel] [PATCH 0/2] migration/postcopy: simplify postcopy_chunk_hostpages_pass Wei Yang 2019-08-06 0:46 ` [Qemu-devel] [PATCH 1/2] migration/postcopy: simplify calculation of run_start and fixup_start_addr Wei Yang 2019-08-06 0:46 ` [Qemu-devel] [PATCH 2/2] migration/postcopy: use QEMU_IS_ALIGNED to replace host_offset Wei Yang @ 2019-08-07 18:43 ` Dr. David Alan Gilbert 2 siblings, 0 replies; 7+ messages in thread From: Dr. David Alan Gilbert @ 2019-08-07 18:43 UTC (permalink / raw) To: Wei Yang; +Cc: qemu-devel, quintela * Wei Yang (richardw.yang@linux.intel.com) wrote: > When looking into function postcopy_chunk_hostpages_pass(), we could use > alignment calculation to simplify it. > > Wei Yang (2): > migration/postcopy: simplify calculation of run_start and > fixup_start_addr > migration/postcopy: use QEMU_IS_ALIGNED to replace host_offset > > migration/ram.c | 37 +++++++------------------------------ > 1 file changed, 7 insertions(+), 30 deletions(-) Queued > -- > 2.17.1 > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-08-07 18:44 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-06 0:46 [Qemu-devel] [PATCH 0/2] migration/postcopy: simplify postcopy_chunk_hostpages_pass Wei Yang 2019-08-06 0:46 ` [Qemu-devel] [PATCH 1/2] migration/postcopy: simplify calculation of run_start and fixup_start_addr Wei Yang 2019-08-06 14:51 ` Dr. David Alan Gilbert 2019-08-07 0:14 ` Wei Yang 2019-08-06 0:46 ` [Qemu-devel] [PATCH 2/2] migration/postcopy: use QEMU_IS_ALIGNED to replace host_offset Wei Yang 2019-08-06 15:47 ` Dr. David Alan Gilbert 2019-08-07 18:43 ` [Qemu-devel] [PATCH 0/2] migration/postcopy: simplify postcopy_chunk_hostpages_pass Dr. David Alan Gilbert
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).