Re: [Qemu-devel] [PATCH 1/2] migration/postcopy: simplify calculation of run_start and fixup_start_addr

[Dr. David Alan Gilbert]

* Wei Yang (address@hidden) 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 <address@hidden>

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 <address@hidden>


> ---
>  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 / address@hidden / Manchester, UK