[PATCH RFC 01/15] migration: No off-by-one for pss->page update in host page size

Peter Xu posted 15 patches 4 years ago
Maintainers: Juan Quintela <quintela@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Thomas Huth <thuth@redhat.com>, Markus Armbruster <armbru@redhat.com>, Eric Blake <eblake@redhat.com>, Laurent Vivier <lvivier@redhat.com>
There is a newer version of this series
[PATCH RFC 01/15] migration: No off-by-one for pss->page update in host page size
Posted by Peter Xu 4 years ago
We used to do off-by-one fixup for pss->page when finished one host huge page
transfer.  That seems to be unnecesary at all.  Drop it.

Cc: Keqian Zhu <zhukeqian1@huawei.com>
Cc: Kunkun Jiang <jiangkunkun@huawei.com>
Cc: Andrey Gruzdev <andrey.gruzdev@virtuozzo.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/ram.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 5234d1ece1..381ad56d26 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1611,7 +1611,7 @@ static int ram_save_release_protection(RAMState *rs, PageSearchStatus *pss,
     /* Check if page is from UFFD-managed region. */
     if (pss->block->flags & RAM_UF_WRITEPROTECT) {
         void *page_address = pss->block->host + (start_page << TARGET_PAGE_BITS);
-        uint64_t run_length = (pss->page - start_page + 1) << TARGET_PAGE_BITS;
+        uint64_t run_length = (pss->page - start_page) << TARGET_PAGE_BITS;
 
         /* Flush async buffers before un-protect. */
         qemu_fflush(rs->f);
@@ -2230,7 +2230,7 @@ static int ram_save_host_page(RAMState *rs, PageSearchStatus *pss,
              offset_in_ramblock(pss->block,
                                 ((ram_addr_t)pss->page) << TARGET_PAGE_BITS));
     /* The offset we leave with is the min boundary of host page and block */
-    pss->page = MIN(pss->page, hostpage_boundary) - 1;
+    pss->page = MIN(pss->page, hostpage_boundary);
 
     res = ram_save_release_protection(rs, pss, start_page);
     return (res < 0 ? res : pages);
-- 
2.32.0


Re: [PATCH RFC 01/15] migration: No off-by-one for pss->page update in host page size
Posted by Dr. David Alan Gilbert 4 years ago
* Peter Xu (peterx@redhat.com) wrote:
> We used to do off-by-one fixup for pss->page when finished one host huge page
> transfer.  That seems to be unnecesary at all.  Drop it.
> 
> Cc: Keqian Zhu <zhukeqian1@huawei.com>
> Cc: Kunkun Jiang <jiangkunkun@huawei.com>
> Cc: Andrey Gruzdev <andrey.gruzdev@virtuozzo.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>

Yes, I think so - I guess the -1 and +1 cancel so it works, and in
practice ram_save_host_page then points to 1 page inside the hugepage
which is then always clean (because it just sent it) so probably
survives.

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

> ---
>  migration/ram.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 5234d1ece1..381ad56d26 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1611,7 +1611,7 @@ static int ram_save_release_protection(RAMState *rs, PageSearchStatus *pss,
>      /* Check if page is from UFFD-managed region. */
>      if (pss->block->flags & RAM_UF_WRITEPROTECT) {
>          void *page_address = pss->block->host + (start_page << TARGET_PAGE_BITS);
> -        uint64_t run_length = (pss->page - start_page + 1) << TARGET_PAGE_BITS;
> +        uint64_t run_length = (pss->page - start_page) << TARGET_PAGE_BITS;
>  
>          /* Flush async buffers before un-protect. */
>          qemu_fflush(rs->f);
> @@ -2230,7 +2230,7 @@ static int ram_save_host_page(RAMState *rs, PageSearchStatus *pss,
>               offset_in_ramblock(pss->block,
>                                  ((ram_addr_t)pss->page) << TARGET_PAGE_BITS));
>      /* The offset we leave with is the min boundary of host page and block */
> -    pss->page = MIN(pss->page, hostpage_boundary) - 1;
> +    pss->page = MIN(pss->page, hostpage_boundary);
>  
>      res = ram_save_release_protection(rs, pss, start_page);
>      return (res < 0 ? res : pages);
> -- 
> 2.32.0
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


Re: [PATCH RFC 01/15] migration: No off-by-one for pss->page update in host page size
Posted by Juan Quintela 4 years ago
Peter Xu <peterx@redhat.com> wrote:
> We used to do off-by-one fixup for pss->page when finished one host huge page
> transfer.  That seems to be unnecesary at all.  Drop it.
>
> Cc: Keqian Zhu <zhukeqian1@huawei.com>
> Cc: Kunkun Jiang <jiangkunkun@huawei.com>
> Cc: Andrey Gruzdev <andrey.gruzdev@virtuozzo.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>


Reviewed-by: Juan Quintela <quintela@redhat.com>

queued.