[libvirt] [PATCH 1/3] virNumaGetPageInfo: Take huge pages into account

[Michal Privoznik]

On 24.06.2014 10:15, Ján Tomko wrote:
> On 06/23/2014 03:59 PM, Michal Privoznik wrote:
>> On the Linux kernel, if huge pages are allocated the size they cut off
>> from memory is accounted under the 'MemUsed' in the meminfo file.
>> However, we want the sum to be subtracted from 'MemTotal'. This patch
>> implements this feature. After this change, we can enable reporting
>> of the ordinary system pages in the capability XML:
>>
>> <capabilities>
>>
>>    <host>
>>      <uuid>01281cda-f352-cb11-a9db-e905fe22010c</uuid>
>>      <cpu>
>>        <arch>x86_64</arch>
>>        <model>Haswell</model>
>>        <vendor>Intel</vendor>
>>        <topology sockets='1' cores='1' threads='1'/>
>>        <feature/>
>>        <pages unit='KiB' size='4'/>
>>        <pages unit='KiB' size='2048'/>
>>        <pages unit='KiB' size='1048576'/>
>>      </cpu>
>>      <power_management/>
>>      <migration_features/>
>>      <topology>
>>        <cells num='4'>
>>          <cell id='0'>
>>            <memory unit='KiB'>4048248</memory>
>>            <pages unit='KiB' size='4'>748382</pages>
>>            <pages unit='KiB' size='2048'>3</pages>
>>            <pages unit='KiB' size='1048576'>1</pages>
>>            <distances/>
>>            <cpus num='1'>
>>              <cpu id='0' socket_id='0' core_id='0' siblings='0'/>
>>            </cpus>
>>          </cell>
>>          ...
>>        </cells>
>>      </topology>
>>    </host>
>> </capabilities>
>>
>> You can see the beautiful thing about this: if you sum up all the
>> <pages/> you'll get <memory/>.
>>
>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>> ---
>>   src/nodeinfo.c     |  2 +-
>>   src/util/virnuma.c | 65 +++++++++++++++++++++++++++++++-----------------------
>>   src/util/virnuma.h |  1 +
>>   3 files changed, 39 insertions(+), 29 deletions(-)
>>
>
>> @@ -647,6 +649,13 @@ virNumaGetHugePageInfo(int node,
>>    * total number of pages in the pool (both free and taken)
>>    * and count for free pages in the pool.
>>    *
>> + * The @huge_page_sum parameter exists there due to the Linux
>
> s/there//

Fixed.

>
>> + * kernel limitation. The problem is, if there are some huge
>> + * pages allocated, they are accounted under the 'MemUsed' field
>> + * in the meminfo file instead of being subtracted from the
>> + * 'MemTotal'. We must do the subtraction ourselves.
>> + * If unsure, pass 0.
>> + *
>>    * If you're interested in just one bit, pass NULL to the other one.
>>    *
>>    * As a special case, if @node == -1, overall info is fetched
>> @@ -657,6 +666,7 @@ virNumaGetHugePageInfo(int node,
>>   int
>>   virNumaGetPageInfo(int node,
>>                      unsigned int page_size,
>> +                   unsigned long long huge_page_sum,
>>                      unsigned int *page_avail,
>>                      unsigned int *page_free)
>>   {
>> @@ -666,7 +676,6 @@ 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
>>           unsigned long long memsize, memfree;
>>
>>           /* TODO: come up with better algorithm that takes huge pages into
>> @@ -679,16 +688,14 @@ virNumaGetPageInfo(int node,
>>                   goto cleanup;
>>           }
>>
>> +        /* see description above */
>
> I think you can just move the comment above here.

This one I'd really like in the function description. I mean, the 
@huge_page_sum is a function argument which certainly deserves a 
description and the aim of documenting internal functions is to not 
force other programmers to actually read the function code. So I'm 
keeping it there.

>
>> +        memsize -= huge_page_sum;
>> +
>>           if (page_avail)
>>               *page_avail = memsize / system_page_size;
>>
>>           if (page_free)
>>               *page_free = memfree / system_page_size;
>
> ACK
>
> Jan
>

Michal