[libvirt] [PATCH 0/2] util: numa: Fix few issues with hugepage code
Peter Krempa (2): util: numa: Catch readdir errors in virNumaGetPages util: numa: Stub out hugepage code on non-Linux platforms src/util/virnuma.c | 52 +++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 9 deletions(-) -- 1.9.3
Don't return possibly incomplete result if virDirRead fails. --- src/util/virnuma.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/util/virnuma.c b/src/util/virnuma.c index c8e7f40..9cf5a75 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -728,6 +728,7 @@ virNumaGetPages(int node, int ret = -1; char *path = NULL; DIR *dir = NULL; + int direrr; struct dirent *entry; unsigned int *tmp_size = NULL, *tmp_avail = NULL, *tmp_free = NULL; unsigned int ntmp = 0; @@ -768,7 +769,7 @@ virNumaGetPages(int node, goto cleanup; } - while (virDirRead(dir, &entry, path) > 0) { + while ((direrr = virDirRead(dir, &entry, path)) > 0) { const char *page_name = entry->d_name; unsigned int page_size, page_avail = 0, page_free = 0; char *end; @@ -805,6 +806,9 @@ virNumaGetPages(int node, ntmp++; } + if (direrr < 0) + goto cleanup; + /* Just to produce nice output, sort the arrays by increasing page size */ do { exchange = false; -- 1.9.3
The hugepage sizing and counting code gathers the information from sysfs and thus isn't portable. Stub it out for non-Linux so that we can report a better error. This patch also avoids calling sysinfo() on Mingw where it isn't supported. --- src/util/virnuma.c | 46 ++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/src/util/virnuma.c b/src/util/virnuma.c index 9cf5a75..9cf9686 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -512,9 +512,12 @@ virNumaGetDistances(int node ATTRIBUTE_UNUSED, #endif -#define HUGEPAGES_NUMA_PREFIX "/sys/devices/system/node/" -#define HUGEPAGES_SYSTEM_PREFIX "/sys/kernel/mm/hugepages/" -#define HUGEPAGES_PREFIX "hugepages-" +/* currently all the hugepage stuff below is linux only */ +#if WITH_LINUX + +# define HUGEPAGES_NUMA_PREFIX "/sys/devices/system/node/" +# define HUGEPAGES_SYSTEM_PREFIX "/sys/kernel/mm/hugepages/" +# define HUGEPAGES_PREFIX "hugepages-" static int virNumaGetHugePageInfoPath(char **path, @@ -663,7 +666,7 @@ virNumaGetPageInfo(int node, /* sysconf() returns page size in bytes, * the @page_size is however in kibibytes */ if (page_size == system_page_size / 1024) { -#if 0 +# if 0 unsigned long long memsize, memfree; /* TODO: come up with better algorithm that takes huge pages into @@ -681,11 +684,11 @@ virNumaGetPageInfo(int node, if (page_free) *page_free = memfree / system_page_size; -#else +# else virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("system page size are not supported yet")); goto cleanup; -#endif /* 0 */ +# endif /* 0 */ } else { if (virNumaGetHugePageInfo(node, page_size, page_avail, page_free) < 0) goto cleanup; @@ -735,7 +738,7 @@ virNumaGetPages(int node, size_t i; bool exchange; -#if 0 +# if 0 /* This has to be disabled until the time the issue in * virNumaGetPageInfo is resolved. Sorry. */ long system_page_size; @@ -756,7 +759,7 @@ virNumaGetPages(int node, goto cleanup; tmp_size[ntmp] = system_page_size; ntmp++; -#endif /* 0 */ +# endif /* 0 */ /* Now that we got ordinary system pages, lets get info on huge pages */ if (virNumaGetHugePageInfoPath(&path, node, 0, NULL) < 0) @@ -844,3 +847,30 @@ virNumaGetPages(int node, VIR_FREE(path); return ret; } + + +#else /* #if WITH_LINUX */ +int +virNumaGetPageInfo(int node ATTRIBUTE_UNUSED, + unsigned int page_size ATTRIBUTE_UNUSED, + unsigned int *page_avail ATTRIBUTE_UNUSED, + unsigned int *page_free ATTRIBUTE_UNUSED) +{ + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("system page size are not supported for this platform")); + return -1; +} + + +int +virNumaGetPages(int node ATTRIBUTE_UNUSED, + unsigned int **pages_size ATTRIBUTE_UNUSED, + unsigned int **pages_avail ATTRIBUTE_UNUSED, + unsigned int **pages_free ATTRIBUTE_UNUSED, + size_t *npages ATTRIBUTE_UNUSED) +{ + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("system page size are not supported for this platform")); + return -1; +} +#endif /* #if WITH_LINUX */ -- 1.9.3
On 23.06.2014 09:29, Peter Krempa wrote:
The hugepage sizing and counting code gathers the information from sysfs and thus isn't portable. Stub it out for non-Linux so that we can report a better error. This patch also avoids calling sysinfo() on Mingw where it isn't supported. --- src/util/virnuma.c | 46 ++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 38 insertions(+), 8 deletions(-)
diff --git a/src/util/virnuma.c b/src/util/virnuma.c index 9cf5a75..9cf9686 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -512,9 +512,12 @@ virNumaGetDistances(int node ATTRIBUTE_UNUSED, #endif
-#define HUGEPAGES_NUMA_PREFIX "/sys/devices/system/node/" -#define HUGEPAGES_SYSTEM_PREFIX "/sys/kernel/mm/hugepages/" -#define HUGEPAGES_PREFIX "hugepages-" +/* currently all the hugepage stuff below is linux only */ +#if WITH_LINUX + +# define HUGEPAGES_NUMA_PREFIX "/sys/devices/system/node/" +# define HUGEPAGES_SYSTEM_PREFIX "/sys/kernel/mm/hugepages/" +# define HUGEPAGES_PREFIX "hugepages-"
static int virNumaGetHugePageInfoPath(char **path, @@ -663,7 +666,7 @@ virNumaGetPageInfo(int node, /* sysconf() returns page size in bytes, * the @page_size is however in kibibytes */ if (page_size == system_page_size / 1024) { -#if 0 +# if 0 unsigned long long memsize, memfree;
/* TODO: come up with better algorithm that takes huge pages into @@ -681,11 +684,11 @@ virNumaGetPageInfo(int node,
if (page_free) *page_free = memfree / system_page_size; -#else +# else virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("system page size are not supported yet")); goto cleanup; -#endif /* 0 */ +# endif /* 0 */ } else { if (virNumaGetHugePageInfo(node, page_size, page_avail, page_free) < 0) goto cleanup; @@ -735,7 +738,7 @@ virNumaGetPages(int node, size_t i; bool exchange;
-#if 0 +# if 0 /* This has to be disabled until the time the issue in * virNumaGetPageInfo is resolved. Sorry. */ long system_page_size; @@ -756,7 +759,7 @@ virNumaGetPages(int node, goto cleanup; tmp_size[ntmp] = system_page_size; ntmp++; -#endif /* 0 */ +# endif /* 0 */
/* Now that we got ordinary system pages, lets get info on huge pages */ if (virNumaGetHugePageInfoPath(&path, node, 0, NULL) < 0) @@ -844,3 +847,30 @@ virNumaGetPages(int node, VIR_FREE(path); return ret;
Up till here everything's okay. But ...
} + + +#else /* #if WITH_LINUX */ +int +virNumaGetPageInfo(int node ATTRIBUTE_UNUSED, + unsigned int page_size ATTRIBUTE_UNUSED, + unsigned int *page_avail ATTRIBUTE_UNUSED, + unsigned int *page_free ATTRIBUTE_UNUSED) +{ + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("system page size are not supported for this platform")); + return -1; +} + + +int +virNumaGetPages(int node ATTRIBUTE_UNUSED, + unsigned int **pages_size ATTRIBUTE_UNUSED, + unsigned int **pages_avail ATTRIBUTE_UNUSED, + unsigned int **pages_free ATTRIBUTE_UNUSED, + size_t *npages ATTRIBUTE_UNUSED) +{ + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("system page size are not supported for this platform")); + return -1; +} +#endif /* #if WITH_LINUX */
These two APIs are intended to get info for all page sizes, not only the ordinary system ones. ACK with the error message changed to reflect that. Michal
On 06/23/14 13:49, Michal Privoznik wrote:
On 23.06.2014 09:29, Peter Krempa wrote:
The hugepage sizing and counting code gathers the information from sysfs and thus isn't portable. Stub it out for non-Linux so that we can report a better error. This patch also avoids calling sysinfo() on Mingw where it isn't supported. --- src/util/virnuma.c | 46 ++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 38 insertions(+), 8 deletions(-)
+int +virNumaGetPages(int node ATTRIBUTE_UNUSED, + unsigned int **pages_size ATTRIBUTE_UNUSED, + unsigned int **pages_avail ATTRIBUTE_UNUSED, + unsigned int **pages_free ATTRIBUTE_UNUSED, + size_t *npages ATTRIBUTE_UNUSED) +{ + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("system page size are not supported for this platform")); + return -1; +} +#endif /* #if WITH_LINUX */
These two APIs are intended to get info for all page sizes, not only the ordinary system ones.
ACK with the error message changed to reflect that.
I'm going with "page info is not supported on this platform" and VIR_ERR_OPERATION_UNSUPPORTED code.
Michal
And pushing shortly. Peter
On 23.6.2014 09:29, Peter Krempa wrote:
Peter Krempa (2): util: numa: Catch readdir errors in virNumaGetPages util: numa: Stub out hugepage code on non-Linux platforms
src/util/virnuma.c | 52 +++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 9 deletions(-)
ACK series, mingw build tested and it works Pavel
participants (3)
-
Michal Privoznik -
Pavel Hrdina -
Peter Krempa