On Fri, Apr 23, 2021 at 15:24:23 +0200, Michal Privoznik wrote:
> New virHostMemGetTHPSize() is introduced which allows caller to
> obtain THP PMD (Page Middle Directory) size, which is equal to
> the minimal size that THP can use, taken from kernel doc
> (Documentation/admin-guide/mm/transhuge.rst):
>
> Some userspace (such as a test program, or an optimized memory allocation
> library) may want to know the size (in bytes) of a transparent hugepage::
>
> cat /sys/kernel/mm/transparent_hugepage/hpage_pmd_size
>
> Since this size depends on the host architecture and the kernel
> it won't change whilst libvirtd is running. Therefore, we can use
> virOnce() and cache the value. Of course, we can be running under
> kernel that has THP disabled or has no notion of THP at all. In
> that case a negative value is returned to signal error.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> src/libvirt_private.syms | 1 +
> src/util/virhostmem.c | 63 ++++++++++++++++++++++++++++++++++++++++
> src/util/virhostmem.h | 3 ++
> tests/domaincapsmock.c | 9 ++++++
> 4 files changed, 76 insertions(+)
[...]
> diff --git a/src/util/virhostmem.c b/src/util/virhostmem.c
> index ae42978ed2..89b31af3ca 100644
> --- a/src/util/virhostmem.c
> +++ b/src/util/virhostmem.c
> @@ -45,11 +45,14 @@
> #include "virstring.h"
> #include "virnuma.h"
> #include "virlog.h"
> +#include "virthread.h"
>
> #define VIR_FROM_THIS VIR_FROM_NONE
>
> VIR_LOG_INIT("util.hostmem");
>
> +static unsigned long long virHostTHPPMDSize; /* in kibibytes */
> +static virOnceControl virHostMemGetTHPSizeOnce = VIR_ONCE_CONTROL_INITIALIZER;
>
> #ifdef __FreeBSD__
> # define BSD_MEMORY_STATS_ALL 4
> @@ -920,3 +923,63 @@ virHostMemAllocPages(unsigned int npages,
>
> return ncounts;
> }
> +
> +#if defined(__linux__)
> +# define HPAGE_PMD_SIZE_PATH "/sys/kernel/mm/transparent_hugepage/hpage_pmd_size"
> +static int
Nothing checks the return value.
> +virHostMemGetTHPSizeSysfs(unsigned long long *size)
> +{
> + g_autofree char *buf = NULL;
> +
> + /* 1KiB limit is more than enough. */
> + if (virFileReadAll(HPAGE_PMD_SIZE_PATH, 1024, &buf) < 0)
> + return -1;
> +
> + virStringTrimOptionalNewline(buf);
> + if (virStrToLong_ull(buf, NULL, 10, size) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("unable to parse THP PMD size: %s"), buf);
> + return -1;
> + }
> +
> + /* Size is now in bytes. Convert to KiB. */
> + *size >>= 10;
> + return 0;
> +}
> +#endif /* defined(__linux__) */
> +
> +
> +static void
> +virHostMemGetTHPSizeOnceInit(void)
> +{
> +#if defined(__linux__)
> + virHostMemGetTHPSizeSysfs(&virHostTHPPMDSize);
> +#else /* !defined(__linux__) */
> + VIR_WARN("Getting THP size not ported yet");
> +#endif /* !defined(__linux__) */
> +}
> +
> +
> +/**
> + * virHostMemGetTHPSize:
> + * @size: returned size of THP in kibibytes
> + *
> + * Obtain Transparent Huge Page size in kibibytes. The size
> + * depends on host architecture and kernel. Because of virOnce(),
> + * do not rely on errno in case of failure.
> + *
> + * Returns: 0 on success,
> + * -1 on failure.
> + */
> +int
> +virHostMemGetTHPSize(unsigned long long *size)
> +{
> + if (virOnce(&virHostMemGetTHPSizeOnce, virHostMemGetTHPSizeOnceInit) < 0)
This directly returns the return value from 'pthread_once' whose manual
entry states:
RETURN VALUE
Upon successful completion, pthread_once() shall return zero; otherwise, an error number shall be returned to indicate the error.
Which reads as if 'errno' is returned by the function which would make
the '< 0' check wrong.
> + return -1;
> +
> + if (virHostTHPPMDSize == 0)
> + return -1;
> +
> + *size = virHostTHPPMDSize;
> + return 0;
> +}
With the above problems addressed:
Reviewed-by: Peter Krempa <pkrempa@xxxxxxxxxx>
