[Qemu-devel] [PATCH 0/3] migration/postcopy: cleanup function postcopy_send_discard_bm_ram

[Qemu-devel] [PATCH 0/3] migration/postcopy: cleanup function postcopy_send_discard_bm_ram

[Wei Yang]

Some cleanup of function postcopy_send_discard_bm_ram:

* use a more restrict check for discard page
* break the loop when no more page to discard
* it is for sure discard_length is not 0

No functional change.

Wei Yang (3):
  migration/postcopy: the valid condition is one less then end
  migration/postcopy: break the loop when there is no more page to
    discard
  migration/postcopy: discard_length must not be 0

 migration/ram.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

-- 
2.19.1

[Qemu-devel] [PATCH 1/3] migration/postcopy: the valid condition is one less then end

[Wei Yang]

If one equals end, it means we have gone through the whole bitmap.

Use a more restrict check to skip a unnecessary condition.

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

diff --git a/migration/ram.c b/migration/ram.c
index 908517fc2b..b78169e811 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2777,7 +2777,7 @@ static int postcopy_send_discard_bm_ram(MigrationState *ms,
     for (current = 0; current < end; ) {
         unsigned long one = find_next_bit(unsentmap, end, current);
 
-        if (one <= end) {
+        if (one < end) {
             unsigned long zero = find_next_zero_bit(unsentmap, end, one + 1);
             unsigned long discard_length;
 
-- 
2.19.1

Re: [Qemu-devel] [PATCH 1/3] migration/postcopy: the valid condition is one less then end

[Dr. David Alan Gilbert]

* Wei Yang (richardw.yang@linux.intel.com) wrote:
> If one equals end, it means we have gone through the whole bitmap.
> 
> Use a more restrict check to skip a unnecessary condition.
> 
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>

Yes, I don't think that'll break, since I think the find_next_zero_bit
will also return end, and then discard_length would be 0; still I think
it's a correct fix.

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

> ---
>  migration/ram.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 908517fc2b..b78169e811 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2777,7 +2777,7 @@ static int postcopy_send_discard_bm_ram(MigrationState *ms,
>      for (current = 0; current < end; ) {
>          unsigned long one = find_next_bit(unsentmap, end, current);
>  
> -        if (one <= end) {
> +        if (one < end) {
>              unsigned long zero = find_next_zero_bit(unsentmap, end, one + 1);
>              unsigned long discard_length;
>  
> -- 
> 2.19.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 1/3] migration/postcopy: the valid condition is one less then end

[Wei Yang]

On Fri, Jun 28, 2019 at 04:09:50PM +0100, Dr. David Alan Gilbert wrote:
>* Wei Yang (richardw.yang@linux.intel.com) wrote:
>> If one equals end, it means we have gone through the whole bitmap.
>> 
>> Use a more restrict check to skip a unnecessary condition.
>> 
>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>
>Yes, I don't think that'll break, since I think the find_next_zero_bit
>will also return end, and then discard_length would be 0; still I think
>it's a correct fix.

Yep, you are exactly right.

>
>Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
>> ---
>>  migration/ram.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/migration/ram.c b/migration/ram.c
>> index 908517fc2b..b78169e811 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -2777,7 +2777,7 @@ static int postcopy_send_discard_bm_ram(MigrationState *ms,
>>      for (current = 0; current < end; ) {
>>          unsigned long one = find_next_bit(unsentmap, end, current);
>>  
>> -        if (one <= end) {
>> +        if (one < end) {
>>              unsigned long zero = find_next_zero_bit(unsentmap, end, one + 1);
>>              unsigned long discard_length;
>>  
>> -- 
>> 2.19.1
>> 
>--
>Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

-- 
Wei Yang
Help you, Help me

[Qemu-devel] [PATCH 2/3] migration/postcopy: break the loop when there is no more page to discard

[Wei Yang]

When one is equal or bigger then end, it means there is no page to
discard. Just break the loop in this case instead of processing it.

No functional change, just refactor it a little.

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

diff --git a/migration/ram.c b/migration/ram.c
index b78169e811..b41b58ee54 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2776,23 +2776,23 @@ static int postcopy_send_discard_bm_ram(MigrationState *ms,
 
     for (current = 0; current < end; ) {
         unsigned long one = find_next_bit(unsentmap, end, current);
+        unsigned long zero, discard_length;
 
-        if (one < end) {
-            unsigned long zero = find_next_zero_bit(unsentmap, end, one + 1);
-            unsigned long discard_length;
+        if (one >= end) {
+            break;
+        }
 
-            if (zero >= end) {
-                discard_length = end - one;
-            } else {
-                discard_length = zero - one;
-            }
-            if (discard_length) {
-                postcopy_discard_send_range(ms, pds, one, discard_length);
-            }
-            current = one + discard_length;
+        zero = find_next_zero_bit(unsentmap, end, one + 1);
+
+        if (zero >= end) {
+            discard_length = end - one;
         } else {
-            current = one;
+            discard_length = zero - one;
+        }
+        if (discard_length) {
+            postcopy_discard_send_range(ms, pds, one, discard_length);
         }
+        current = one + discard_length;
     }
 
     return 0;
-- 
2.19.1

Re: [Qemu-devel] [PATCH 2/3] migration/postcopy: break the loop when there is no more page to discard

[Dr. David Alan Gilbert]

* Wei Yang (richardw.yang@linux.intel.com) wrote:
> When one is equal or bigger then end, it means there is no page to
> discard. Just break the loop in this case instead of processing it.
> 
> No functional change, just refactor it a little.
> 
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>

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

> ---
>  migration/ram.c | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index b78169e811..b41b58ee54 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2776,23 +2776,23 @@ static int postcopy_send_discard_bm_ram(MigrationState *ms,
>  
>      for (current = 0; current < end; ) {
>          unsigned long one = find_next_bit(unsentmap, end, current);
> +        unsigned long zero, discard_length;
>  
> -        if (one < end) {
> -            unsigned long zero = find_next_zero_bit(unsentmap, end, one + 1);
> -            unsigned long discard_length;
> +        if (one >= end) {
> +            break;
> +        }
>  
> -            if (zero >= end) {
> -                discard_length = end - one;
> -            } else {
> -                discard_length = zero - one;
> -            }
> -            if (discard_length) {
> -                postcopy_discard_send_range(ms, pds, one, discard_length);
> -            }
> -            current = one + discard_length;
> +        zero = find_next_zero_bit(unsentmap, end, one + 1);
> +
> +        if (zero >= end) {
> +            discard_length = end - one;
>          } else {
> -            current = one;
> +            discard_length = zero - one;
> +        }
> +        if (discard_length) {
> +            postcopy_discard_send_range(ms, pds, one, discard_length);
>          }
> +        current = one + discard_length;
>      }
>  
>      return 0;
> -- 
> 2.19.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

[Qemu-devel] [PATCH 3/3] migration/postcopy: discard_length must not be 0

[Wei Yang]

Since we break the loop when there is no more page to discard, we are
sure the following process would find some page to discard.

It is not necessary to check it again.

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

diff --git a/migration/ram.c b/migration/ram.c
index b41b58ee54..246efe6939 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2789,9 +2789,7 @@ static int postcopy_send_discard_bm_ram(MigrationState *ms,
         } else {
             discard_length = zero - one;
         }
-        if (discard_length) {
-            postcopy_discard_send_range(ms, pds, one, discard_length);
-        }
+        postcopy_discard_send_range(ms, pds, one, discard_length);
         current = one + discard_length;
     }
 
-- 
2.19.1

Re: [Qemu-devel] [PATCH 3/3] migration/postcopy: discard_length must not be 0

[Dr. David Alan Gilbert]

* Wei Yang (richardw.yang@linux.intel.com) wrote:
> Since we break the loop when there is no more page to discard, we are
> sure the following process would find some page to discard.
> 
> It is not necessary to check it again.
> 
> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>

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

> ---
>  migration/ram.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index b41b58ee54..246efe6939 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2789,9 +2789,7 @@ static int postcopy_send_discard_bm_ram(MigrationState *ms,
>          } else {
>              discard_length = zero - one;
>          }
> -        if (discard_length) {
> -            postcopy_discard_send_range(ms, pds, one, discard_length);
> -        }
> +        postcopy_discard_send_range(ms, pds, one, discard_length);
>          current = one + discard_length;
>      }
>  
> -- 
> 2.19.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 0/3] migration/postcopy: cleanup function postcopy_send_discard_bm_ram

[Dr. David Alan Gilbert]

* Wei Yang (richardw.yang@linux.intel.com) wrote:
> Some cleanup of function postcopy_send_discard_bm_ram:
> 
> * use a more restrict check for discard page
> * break the loop when no more page to discard
> * it is for sure discard_length is not 0
> 
> No functional change.
> 

Queued

> Wei Yang (3):
>   migration/postcopy: the valid condition is one less then end
>   migration/postcopy: break the loop when there is no more page to
>     discard
>   migration/postcopy: discard_length must not be 0
> 
>  migration/ram.c | 24 +++++++++++-------------
>  1 file changed, 11 insertions(+), 13 deletions(-)
> 
> -- 
> 2.19.1
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK