diff mbox

[2/2] migration/postcopy: Explicitly disallow huge pages

Message ID 1475176178-4873-3-git-send-email-dgilbert@redhat.com
State New
Headers show

Commit Message

Dr. David Alan Gilbert Sept. 29, 2016, 7:09 p.m. UTC
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

At the moment postcopy will fail as soon as qemu tries to register
userfault on the RAMBlock pages that are backed by hugepages.
However, the kernel is going to get userfault support for hugepage
at some point, and we've not got the rest of the QEMU code to support
it yet, so fail neatly with an error like:

Postcopy doesn't support hugetlbfs yet (/objects/mem1)

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/postcopy-ram.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

Comments

Daniel P. Berrangé Sept. 30, 2016, 8:09 a.m. UTC | #1
On Thu, Sep 29, 2016 at 08:09:38PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> At the moment postcopy will fail as soon as qemu tries to register
> userfault on the RAMBlock pages that are backed by hugepages.
> However, the kernel is going to get userfault support for hugepage
> at some point, and we've not got the rest of the QEMU code to support
> it yet, so fail neatly with an error like:
> 
> Postcopy doesn't support hugetlbfs yet (/objects/mem1)
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  migration/postcopy-ram.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index 9b04778..9723593 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -85,6 +85,23 @@ static bool ufd_version_check(int ufd)
>  }
>  
>  /*
> + * Check for things that postcopy won't support; returns 0 if the block
> + * is fine.
> + */
> +static int check_range(const char *block_name, void *host_addr,
> +                      ram_addr_t offset, ram_addr_t length, void *opaque)
> +{
> +    RAMBlock *rb = qemu_ram_block_by_name(block_name);
> +
> +    if (qemu_ram_pagesize(rb) > getpagesize()) {
> +        error_report("Postcopy doesn't support hugetlbfs yet (%s)", block_name);

A small nitpick - I'd suggest  s/hugetlbfs/large page sizes/ as this error
will ultimately bubble up to users, and 'large page sizes' is the conceptual
feature


Regards,
Daniel
Dr. David Alan Gilbert Oct. 5, 2016, 9:54 a.m. UTC | #2
* Daniel P. Berrange (berrange@redhat.com) wrote:
> On Thu, Sep 29, 2016 at 08:09:38PM +0100, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > At the moment postcopy will fail as soon as qemu tries to register
> > userfault on the RAMBlock pages that are backed by hugepages.
> > However, the kernel is going to get userfault support for hugepage
> > at some point, and we've not got the rest of the QEMU code to support
> > it yet, so fail neatly with an error like:
> > 
> > Postcopy doesn't support hugetlbfs yet (/objects/mem1)
> > 
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> >  migration/postcopy-ram.c | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> > 
> > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> > index 9b04778..9723593 100644
> > --- a/migration/postcopy-ram.c
> > +++ b/migration/postcopy-ram.c
> > @@ -85,6 +85,23 @@ static bool ufd_version_check(int ufd)
> >  }
> >  
> >  /*
> > + * Check for things that postcopy won't support; returns 0 if the block
> > + * is fine.
> > + */
> > +static int check_range(const char *block_name, void *host_addr,
> > +                      ram_addr_t offset, ram_addr_t length, void *opaque)
> > +{
> > +    RAMBlock *rb = qemu_ram_block_by_name(block_name);
> > +
> > +    if (qemu_ram_pagesize(rb) > getpagesize()) {
> > +        error_report("Postcopy doesn't support hugetlbfs yet (%s)", block_name);
> 
> A small nitpick - I'd suggest  s/hugetlbfs/large page sizes/ as this error
> will ultimately bubble up to users, and 'large page sizes' is the conceptual
> feature

Interesting, I'd never heard it called 'large' before.
Everytime I just say 'huge page' to someone I then have to explain it's fine
on transparent huge page.

Dave

> 
> 
> Regards,
> Daniel
> -- 
> |: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org              -o-             http://virt-manager.org :|
> |: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Juan Quintela Oct. 5, 2016, 10:02 a.m. UTC | #3
"Daniel P. Berrange" <berrange@redhat.com> wrote:
> On Thu, Sep 29, 2016 at 08:09:38PM +0100, Dr. David Alan Gilbert (git) wrote:
>> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>> 
>> At the moment postcopy will fail as soon as qemu tries to register
>> userfault on the RAMBlock pages that are backed by hugepages.
>> However, the kernel is going to get userfault support for hugepage
>> at some point, and we've not got the rest of the QEMU code to support
>> it yet, so fail neatly with an error like:
>> 
>> Postcopy doesn't support hugetlbfs yet (/objects/mem1)
>> 
>> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> ---
>>  migration/postcopy-ram.c | 23 +++++++++++++++++++++++
>>  1 file changed, 23 insertions(+)
>> 
>> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
>> index 9b04778..9723593 100644
>> --- a/migration/postcopy-ram.c
>> +++ b/migration/postcopy-ram.c
>> @@ -85,6 +85,23 @@ static bool ufd_version_check(int ufd)
>>  }
>>  
>>  /*
>> + * Check for things that postcopy won't support; returns 0 if the block
>> + * is fine.
>> + */
>> +static int check_range(const char *block_name, void *host_addr,
>> +                      ram_addr_t offset, ram_addr_t length, void *opaque)
>> +{
>> +    RAMBlock *rb = qemu_ram_block_by_name(block_name);
>> +
>> +    if (qemu_ram_pagesize(rb) > getpagesize()) {
>> +        error_report("Postcopy doesn't support hugetlbfs yet (%s)", block_name);
>
> A small nitpick - I'd suggest  s/hugetlbfs/large page sizes/ as this error
> will ultimately bubble up to users, and 'large page sizes' is the conceptual
> feature

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

I fixed the message my hand
diff mbox

Patch

diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 9b04778..9723593 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -85,6 +85,23 @@  static bool ufd_version_check(int ufd)
 }
 
 /*
+ * Check for things that postcopy won't support; returns 0 if the block
+ * is fine.
+ */
+static int check_range(const char *block_name, void *host_addr,
+                      ram_addr_t offset, ram_addr_t length, void *opaque)
+{
+    RAMBlock *rb = qemu_ram_block_by_name(block_name);
+
+    if (qemu_ram_pagesize(rb) > getpagesize()) {
+        error_report("Postcopy doesn't support hugetlbfs yet (%s)", block_name);
+        return -E2BIG;
+    }
+
+    return 0;
+}
+
+/*
  * Note: This has the side effect of munlock'ing all of RAM, that's
  * normally fine since if the postcopy succeeds it gets turned back on at the
  * end.
@@ -104,6 +121,12 @@  bool postcopy_ram_supported_by_host(void)
         goto out;
     }
 
+    /* Check for anything about the RAMBlocks we don't support */
+    if (qemu_ram_foreach_block(check_range, NULL)) {
+        /* check_range will have printed its own error */
+        goto out;
+    }
+
     ufd = syscall(__NR_userfaultfd, O_CLOEXEC);
     if (ufd == -1) {
         error_report("%s: userfaultfd not available: %s", __func__,