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@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.
>
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
