| Message ID | 20190806004648.8659-2-richardw.yang@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | migration/postcopy: simplify postcopy_chunk_hostpages_pass | expand |
* 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
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
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)) {
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(-)