[PATCH v3 01/14] virhostmem: Introduce virHostMemGetTHPSize()

[Peter Krempa]

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 at redhat.com>
> ---
>  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 at redhat.com>