[PATCH] kvm/qemu: use statfs to determine size of huge pages

[PATCH] kvm/qemu: use statfs to determine size of huge pages

[Joerg Roedel]

The current method of finding out the size of huge pages does not work
reliable anymore. Current Linux supports more than one huge page size
but /proc/meminfo only show one of the supported sizes.
To find out the real page size used can be found by calling statfs. This
patch changes kvm/qemu to use statfs instead of parsing /proc/meminfo.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 qemu/sysemu.h |    2 +-
 qemu/vl.c     |   42 ++++++++++++++++++++----------------------
 2 files changed, 21 insertions(+), 23 deletions(-)

diff --git a/qemu/sysemu.h b/qemu/sysemu.h
index 19464cf..4333495 100644
--- a/qemu/sysemu.h
+++ b/qemu/sysemu.h
@@ -99,7 +99,7 @@ extern int graphic_rotate;
 extern int no_quit;
 extern int semihosting_enabled;
 extern int old_param;
-extern int hpagesize;
+extern long hpagesize;
 extern const char *bootp_filename;
 
 #ifdef USE_KQEMU
diff --git a/qemu/vl.c b/qemu/vl.c
index bbd7aa3..b8c1162 100644
--- a/qemu/vl.c
+++ b/qemu/vl.c
@@ -61,6 +61,7 @@
 #include <sys/ioctl.h>
 #include <sys/resource.h>
 #include <sys/socket.h>
+#include <sys/vfs.h>
 #include <netinet/in.h>
 #include <net/if.h>
 #if defined(__NetBSD__)
@@ -254,7 +255,7 @@ const char *mem_path = NULL;
 #ifdef MAP_POPULATE
 int mem_prealloc = 1;	/* force preallocation of physical target memory */
 #endif
-int hpagesize = 0;
+long hpagesize = 0;
 const char *cpu_vendor_string;
 #ifdef TARGET_ARM
 int old_param = 0;
@@ -4717,32 +4718,29 @@ void qemu_get_launch_info(int *argc, char ***argv, int *opt_daemonize, const cha
 }
 
 #ifdef USE_KVM
-static int gethugepagesize(void)
+
+#define HUGETLBFS_MAGIC       0x958458f6
+
+static long gethugepagesize(const char *path)
 {
-    int ret, fd;
-    char buf[4096];
-    const char *needle = "Hugepagesize:";
-    char *size;
-    unsigned long hugepagesize;
+    struct statfs fs;
+    int ret;
 
-    fd = open("/proc/meminfo", O_RDONLY);
-    if (fd < 0) {
-	perror("open");
-	exit(0);
+    do {
+	    ret = statfs(path, &fs);
+    } while (ret != 0 && errno == EINTR);
+
+    if (ret != 0) {
+	    perror("statfs");
+	    return 0;
     }
 
-    ret = read(fd, buf, sizeof(buf));
-    if (ret < 0) {
-	perror("read");
-	exit(0);
+    if (fs.f_type != HUGETLBFS_MAGIC) {
+	    fprintf(stderr, "Path not on HugeTLBFS: %s\n", path);
+	    return 0;
     }
 
-    size = strstr(buf, needle);
-    if (!size)
-	return 0;
-    size += strlen(needle);
-    hugepagesize = strtol(size, NULL, 0);
-    return hugepagesize;
+    return fs.f_bsize;
 }
 
 static void *alloc_mem_area(size_t memory, unsigned long *len, const char *path)
@@ -4762,7 +4760,7 @@ static void *alloc_mem_area(size_t memory, unsigned long *len, const char *path)
     if (asprintf(&filename, "%s/kvm.XXXXXX", path) == -1)
 	return NULL;
 
-    hpagesize = gethugepagesize() * 1024;
+    hpagesize = gethugepagesize(path);
     if (!hpagesize)
 	return NULL;
 
-- 
1.5.6.4

Re: [PATCH] kvm/qemu: use statfs to determine size of huge pages

[Marcelo Tosatti]

Hi Joerg,

On Thu, Feb 19, 2009 at 04:29:36PM +0100, Joerg Roedel wrote:
> The current method of finding out the size of huge pages does not work
> reliable anymore. Current Linux supports more than one huge page size
> but /proc/meminfo only show one of the supported sizes.
> To find out the real page size used can be found by calling statfs. This
> patch changes kvm/qemu to use statfs instead of parsing /proc/meminfo.
> 
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>

> +    if (fs.f_type != HUGETLBFS_MAGIC) {
> +	    fprintf(stderr, "Path not on HugeTLBFS: %s\n", path);
> +	    return 0;
>      }

Would be nicer to use standard page size (sysconf) in case its not a
hugetlbfs mount, instead of failing. Useful for testing with shmem.

> -    size = strstr(buf, needle);
> -    if (!size)
> -	return 0;
> -    size += strlen(needle);
> -    hugepagesize = strtol(size, NULL, 0);
> -    return hugepagesize;
> +    return fs.f_bsize;
>  }
>  
>  static void *alloc_mem_area(size_t memory, unsigned long *len, const char *path)
> @@ -4762,7 +4760,7 @@ static void *alloc_mem_area(size_t memory, unsigned long *len, const char *path)
>      if (asprintf(&filename, "%s/kvm.XXXXXX", path) == -1)
>  	return NULL;
>  
> -    hpagesize = gethugepagesize() * 1024;
> +    hpagesize = gethugepagesize(path);
>      if (!hpagesize)
>  	return NULL;

Out of curiosity, your immediate reason for this patch is use of
gbpages?

Re: [PATCH] kvm/qemu: use statfs to determine size of huge pages

[Joerg Roedel]

On Fri, Feb 20, 2009 at 09:18:42PM -0300, Marcelo Tosatti wrote:
> Hi Joerg,
> 
> On Thu, Feb 19, 2009 at 04:29:36PM +0100, Joerg Roedel wrote:
> > The current method of finding out the size of huge pages does not work
> > reliable anymore. Current Linux supports more than one huge page size
> > but /proc/meminfo only show one of the supported sizes.
> > To find out the real page size used can be found by calling statfs. This
> > patch changes kvm/qemu to use statfs instead of parsing /proc/meminfo.
> > 
> > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> 
> > +    if (fs.f_type != HUGETLBFS_MAGIC) {
> > +	    fprintf(stderr, "Path not on HugeTLBFS: %s\n", path);
> > +	    return 0;
> >      }
> 
> Would be nicer to use standard page size (sysconf) in case its not a
> hugetlbfs mount, instead of failing. Useful for testing with shmem.
> 
> > -    size = strstr(buf, needle);
> > -    if (!size)
> > -	return 0;
> > -    size += strlen(needle);
> > -    hugepagesize = strtol(size, NULL, 0);
> > -    return hugepagesize;
> > +    return fs.f_bsize;
> >  }
> >  
> >  static void *alloc_mem_area(size_t memory, unsigned long *len, const char *path)
> > @@ -4762,7 +4760,7 @@ static void *alloc_mem_area(size_t memory, unsigned long *len, const char *path)
> >      if (asprintf(&filename, "%s/kvm.XXXXXX", path) == -1)
> >  	return NULL;
> >  
> > -    hpagesize = gethugepagesize() * 1024;
> > +    hpagesize = gethugepagesize(path);
> >      if (!hpagesize)
> >  	return NULL;
> 
> Out of curiosity, your immediate reason for this patch is use of
> gbpages?

Yes. I am currently preparing patches to support gbpages in KVM. I think
they will be ready for submission next week.
The leak bugs were also found when I was debugging the code this week.
My test system has 5 gbpages an leaking one or two of them is _really_
bad ;)

Joerg