1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH v2 0/3] Add bitmap for received pages in postcopy migration
  4. View patch

[Qemu-devel] [PATCH v2 2/3] migration: introduce qemu_ufd_copy_ioctl helper

Alexey Perevalov posted 3 patches 8 years, 4 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v2 2/3] migration: introduce qemu_ufd_copy_ioctl helper
Posted by Alexey Perevalov 8 years, 4 months ago 
Just for placing auxilary operations inside helper,
auxilary operations like: track received pages,
notify about copying operation in futher patches.

Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
---
 migration/postcopy-ram.c | 35 ++++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 743ca6c..38a429a 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -561,6 +561,26 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
     return 0;
 }
 
+static int qemu_ufd_copy_ioctl(int userfault_fd, void *host_addr,
+        void *from_addr, uint64_t pagesize)
+{
+    if (from_addr) {
+        struct uffdio_copy copy_struct;
+        copy_struct.dst = (uint64_t)(uintptr_t)host_addr;
+        copy_struct.src = (uint64_t)(uintptr_t)from_addr;
+        copy_struct.len = pagesize;
+        copy_struct.mode = 0;
+        return ioctl(userfault_fd, UFFDIO_COPY, &copy_struct);
+    } else {
+        struct uffdio_zeropage zero_struct;
+        zero_struct.range.start = (uint64_t)(uintptr_t)host_addr;
+        zero_struct.range.len = pagesize;
+        zero_struct.mode = 0;
+        return ioctl(userfault_fd, UFFDIO_ZEROPAGE, &zero_struct);
+    }
+    return 0;
+}
+
 /*
  * Place a host page (from) at (host) atomically
  * returns 0 on success
@@ -568,20 +588,14 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
 int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
                         RAMBlock *rb)
 {
-    struct uffdio_copy copy_struct;
     size_t pagesize = qemu_ram_pagesize(rb);
 
-    copy_struct.dst = (uint64_t)(uintptr_t)host;
-    copy_struct.src = (uint64_t)(uintptr_t)from;
-    copy_struct.len = pagesize;
-    copy_struct.mode = 0;
-
     /* copy also acks to the kernel waking the stalled thread up
      * TODO: We can inhibit that ack and only do it if it was requested
      * which would be slightly cheaper, but we'd have to be careful
      * of the order of updating our page state.
      */
-    if (ioctl(mis->userfault_fd, UFFDIO_COPY, &copy_struct)) {
+    if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, from, pagesize)) {
         int e = errno;
         error_report("%s: %s copy host: %p from: %p (size: %zd)",
                      __func__, strerror(e), host, from, pagesize);
@@ -605,12 +619,7 @@ int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
 
     pagesize = qemu_ram_pagesize(rb);
     if (pagesize == getpagesize()) {
-        struct uffdio_zeropage zero_struct;
-        zero_struct.range.start = (uint64_t)(uintptr_t)host;
-        zero_struct.range.len = getpagesize();
-        zero_struct.mode = 0;
-
-        if (ioctl(mis->userfault_fd, UFFDIO_ZEROPAGE, &zero_struct)) {
+        if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, 0, getpagesize())) {
             int e = errno;
             error_report("%s: %s zero host: %p",
                          __func__, strerror(e), host);
-- 
1.9.1
Re: [Qemu-devel] [PATCH v2 2/3] migration: introduce qemu_ufd_copy_ioctl helper
Posted by Peter Xu 8 years, 4 months ago 
On Thu, Jun 15, 2017 at 07:36:14PM +0300, Alexey Perevalov wrote:
> Just for placing auxilary operations inside helper,
> auxilary operations like: track received pages,
> notify about copying operation in futher patches.
> 
> Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
> ---
>  migration/postcopy-ram.c | 35 ++++++++++++++++++++++-------------
>  1 file changed, 22 insertions(+), 13 deletions(-)
> 
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index 743ca6c..38a429a 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -561,6 +561,26 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
>      return 0;
>  }
>  
> +static int qemu_ufd_copy_ioctl(int userfault_fd, void *host_addr,
> +        void *from_addr, uint64_t pagesize)
> +{
> +    if (from_addr) {
> +        struct uffdio_copy copy_struct;
> +        copy_struct.dst = (uint64_t)(uintptr_t)host_addr;
> +        copy_struct.src = (uint64_t)(uintptr_t)from_addr;
> +        copy_struct.len = pagesize;
> +        copy_struct.mode = 0;
> +        return ioctl(userfault_fd, UFFDIO_COPY, &copy_struct);
> +    } else {
> +        struct uffdio_zeropage zero_struct;
> +        zero_struct.range.start = (uint64_t)(uintptr_t)host_addr;
> +        zero_struct.range.len = pagesize;
> +        zero_struct.mode = 0;
> +        return ioctl(userfault_fd, UFFDIO_ZEROPAGE, &zero_struct);
> +    }
> +    return 0;

Remove this line?

> +}
> +
>  /*
>   * Place a host page (from) at (host) atomically
>   * returns 0 on success
> @@ -568,20 +588,14 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
>  int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
>                          RAMBlock *rb)
>  {
> -    struct uffdio_copy copy_struct;
>      size_t pagesize = qemu_ram_pagesize(rb);
>  
> -    copy_struct.dst = (uint64_t)(uintptr_t)host;
> -    copy_struct.src = (uint64_t)(uintptr_t)from;
> -    copy_struct.len = pagesize;
> -    copy_struct.mode = 0;
> -
>      /* copy also acks to the kernel waking the stalled thread up
>       * TODO: We can inhibit that ack and only do it if it was requested
>       * which would be slightly cheaper, but we'd have to be careful
>       * of the order of updating our page state.
>       */
> -    if (ioctl(mis->userfault_fd, UFFDIO_COPY, &copy_struct)) {
> +    if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, from, pagesize)) {

(hmm I know "from" won't be zero, but naturally if without the context
 I would doubt it. Shall we add an assert here? You call.)

Besides:

Reviewed-by: Peter Xu <peterx@redhat.com>

Thanks,

>          int e = errno;
>          error_report("%s: %s copy host: %p from: %p (size: %zd)",
>                       __func__, strerror(e), host, from, pagesize);
> @@ -605,12 +619,7 @@ int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
>  
>      pagesize = qemu_ram_pagesize(rb);
>      if (pagesize == getpagesize()) {
> -        struct uffdio_zeropage zero_struct;
> -        zero_struct.range.start = (uint64_t)(uintptr_t)host;
> -        zero_struct.range.len = getpagesize();
> -        zero_struct.mode = 0;
> -
> -        if (ioctl(mis->userfault_fd, UFFDIO_ZEROPAGE, &zero_struct)) {
> +        if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, 0, getpagesize())) {
>              int e = errno;
>              error_report("%s: %s zero host: %p",
>                           __func__, strerror(e), host);
> -- 
> 1.9.1
> 

-- 
Peter Xu
Re: [Qemu-devel] [PATCH v2 2/3] migration: introduce qemu_ufd_copy_ioctl helper
Posted by Dr. David Alan Gilbert 8 years, 4 months ago 
* Peter Xu (peterx@redhat.com) wrote:
> On Thu, Jun 15, 2017 at 07:36:14PM +0300, Alexey Perevalov wrote:
> > Just for placing auxilary operations inside helper,
> > auxilary operations like: track received pages,
> > notify about copying operation in futher patches.
> > 
> > Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
> > ---
> >  migration/postcopy-ram.c | 35 ++++++++++++++++++++++-------------
> >  1 file changed, 22 insertions(+), 13 deletions(-)
> > 
> > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> > index 743ca6c..38a429a 100644
> > --- a/migration/postcopy-ram.c
> > +++ b/migration/postcopy-ram.c
> > @@ -561,6 +561,26 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
> >      return 0;
> >  }
> >  
> > +static int qemu_ufd_copy_ioctl(int userfault_fd, void *host_addr,
> > +        void *from_addr, uint64_t pagesize)
> > +{
> > +    if (from_addr) {
> > +        struct uffdio_copy copy_struct;
> > +        copy_struct.dst = (uint64_t)(uintptr_t)host_addr;
> > +        copy_struct.src = (uint64_t)(uintptr_t)from_addr;
> > +        copy_struct.len = pagesize;
> > +        copy_struct.mode = 0;
> > +        return ioctl(userfault_fd, UFFDIO_COPY, &copy_struct);
> > +    } else {
> > +        struct uffdio_zeropage zero_struct;
> > +        zero_struct.range.start = (uint64_t)(uintptr_t)host_addr;
> > +        zero_struct.range.len = pagesize;
> > +        zero_struct.mode = 0;
> > +        return ioctl(userfault_fd, UFFDIO_ZEROPAGE, &zero_struct);
> > +    }
> > +    return 0;
> 
> Remove this line?

Yes, probably should; only minor.

> > +}
> > +
> >  /*
> >   * Place a host page (from) at (host) atomically
> >   * returns 0 on success
> > @@ -568,20 +588,14 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
> >  int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
> >                          RAMBlock *rb)
> >  {
> > -    struct uffdio_copy copy_struct;
> >      size_t pagesize = qemu_ram_pagesize(rb);
> >  
> > -    copy_struct.dst = (uint64_t)(uintptr_t)host;
> > -    copy_struct.src = (uint64_t)(uintptr_t)from;
> > -    copy_struct.len = pagesize;
> > -    copy_struct.mode = 0;
> > -
> >      /* copy also acks to the kernel waking the stalled thread up
> >       * TODO: We can inhibit that ack and only do it if it was requested
> >       * which would be slightly cheaper, but we'd have to be careful
> >       * of the order of updating our page state.
> >       */
> > -    if (ioctl(mis->userfault_fd, UFFDIO_COPY, &copy_struct)) {
> > +    if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, from, pagesize)) {
> 
> (hmm I know "from" won't be zero, but naturally if without the context
>  I would doubt it. Shall we add an assert here? You call.)

Doesn't seem worth it.

> Besides:
> 
> Reviewed-by: Peter Xu <peterx@redhat.com>


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

> 
> Thanks,
> 
> >          int e = errno;
> >          error_report("%s: %s copy host: %p from: %p (size: %zd)",
> >                       __func__, strerror(e), host, from, pagesize);
> > @@ -605,12 +619,7 @@ int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
> >  
> >      pagesize = qemu_ram_pagesize(rb);
> >      if (pagesize == getpagesize()) {
> > -        struct uffdio_zeropage zero_struct;
> > -        zero_struct.range.start = (uint64_t)(uintptr_t)host;
> > -        zero_struct.range.len = getpagesize();
> > -        zero_struct.mode = 0;
> > -
> > -        if (ioctl(mis->userfault_fd, UFFDIO_ZEROPAGE, &zero_struct)) {
> > +        if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, 0, getpagesize())) {
> >              int e = errno;
> >              error_report("%s: %s zero host: %p",
> >                           __func__, strerror(e), host);
> > -- 
> > 1.9.1
> > 
> 
> -- 
> Peter Xu
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.