[1/3] migration/postcopy: not necessary to do discard when canonicalizing bitmap

All pages, either partially sent or partially dirty, will be discarded in
postcopy_send_discard_bm_ram(), since we update the unsentmap to be
unsentmap = unsentmap | dirty in ram_postcopy_send_discard_bitmap().

This is not necessary to do discard when canonicalizing bitmap. And by
doing so, we separate the page discard into two individual steps:

  * canonicalize bitmap
  * discard page

Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
---
 migration/ram.c | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

* Wei Yang (richardw.yang@linux.intel.com) wrote:
> All pages, either partially sent or partially dirty, will be discarded in
> postcopy_send_discard_bm_ram(), since we update the unsentmap to be
> unsentmap = unsentmap | dirty in ram_postcopy_send_discard_bitmap().
> 
> This is not necessary to do discard when canonicalizing bitmap. And by
> doing so, we separate the page discard into two individual steps:
> 
>   * canonicalize bitmap
>   * discard page
> 
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>

Yes, I think when I originally wrote it, the set of pages that was
discarded was different; I think it was actually the set of 
!unsent & dirty - i.e. only pages that had been sent and then redirtied;
it later got reworked to include unsent pages as well - so this lot can
be simplified.



Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  migration/ram.c | 14 +-------------
>  1 file changed, 1 insertion(+), 13 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 35552c090b..075ddc468c 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2928,7 +2928,7 @@ static int postcopy_each_ram_send_discard(MigrationState *ms)
>  }
>  
>  /**
> - * postcopy_chunk_hostpages_pass: canocalize bitmap in hostpages
> + * postcopy_chunk_hostpages_pass: canonicalize bitmap in hostpages
>   *
>   * Helper for postcopy_chunk_hostpages; it's called twice to
>   * canonicalize the two bitmaps, that are similar, but one is
> @@ -2991,18 +2991,6 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, bool unsent_pass,
>                                                               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)) {
> -                /* For the unsent_pass we:
> -                 *     discard partially sent pages
> -                 * For the !unsent_pass (dirty) we:
> -                 *     discard partially dirty pages that were sent
> -                 *     (any partially sent pages were already discarded
> -                 *     by the previous unsent_pass)
> -                 */
> -                postcopy_discard_send_range(ms, fixup_start_addr, host_ratio);
> -            }
> -
>              /* Clean up the bitmap */
>              for (page = fixup_start_addr;
>                   page < fixup_start_addr + host_ratio; page++) {
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

On Tue, Sep 24, 2019 at 11:02:08AM +0100, Dr. David Alan Gilbert wrote:
>* Wei Yang (richardw.yang@linux.intel.com) wrote:
>> All pages, either partially sent or partially dirty, will be discarded in
>> postcopy_send_discard_bm_ram(), since we update the unsentmap to be
>> unsentmap = unsentmap | dirty in ram_postcopy_send_discard_bitmap().
>> 
>> This is not necessary to do discard when canonicalizing bitmap. And by
>> doing so, we separate the page discard into two individual steps:
>> 
>>   * canonicalize bitmap
>>   * discard page
>> 
>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>
>Yes, I think when I originally wrote it, the set of pages that was
>discarded was different; I think it was actually the set of 
>!unsent & dirty - i.e. only pages that had been sent and then redirtied;
>it later got reworked to include unsent pages as well - so this lot can
>be simplified.
>
>

Thanks for your time :-)

>
>Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
>> ---
>>  migration/ram.c | 14 +-------------
>>  1 file changed, 1 insertion(+), 13 deletions(-)
>> 
>> diff --git a/migration/ram.c b/migration/ram.c
>> index 35552c090b..075ddc468c 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -2928,7 +2928,7 @@ static int postcopy_each_ram_send_discard(MigrationState *ms)
>>  }
>>  
>>  /**
>> - * postcopy_chunk_hostpages_pass: canocalize bitmap in hostpages
>> + * postcopy_chunk_hostpages_pass: canonicalize bitmap in hostpages
>>   *
>>   * Helper for postcopy_chunk_hostpages; it's called twice to
>>   * canonicalize the two bitmaps, that are similar, but one is
>> @@ -2991,18 +2991,6 @@ static void postcopy_chunk_hostpages_pass(MigrationState *ms, bool unsent_pass,
>>                                                               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)) {
>> -                /* For the unsent_pass we:
>> -                 *     discard partially sent pages
>> -                 * For the !unsent_pass (dirty) we:
>> -                 *     discard partially dirty pages that were sent
>> -                 *     (any partially sent pages were already discarded
>> -                 *     by the previous unsent_pass)
>> -                 */
>> -                postcopy_discard_send_range(ms, fixup_start_addr, host_ratio);
>> -            }
>> -
>>              /* Clean up the bitmap */
>>              for (page = fixup_start_addr;
>>                   page < fixup_start_addr + host_ratio; page++) {
>> -- 
>> 2.17.1
>> 
>--
>Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK