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

[Michal Prívozník]

On 5/3/21 1:07 PM, Peter Krempa wrote:
> 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.
> 

Interesting, never though about this. Just blindly copied what we have
elsewhere. Looking into glibc - pthread_once will never return anything
else than 0. Then uclibc-ng doesn't provide pthread_once and musl also
always returns 0. I'll send a patch that fixes this issue shortly.

Michal