[PATCH v2 1/4] numa_conf.c: add helper functions for cpumap operations

Daniel Henrique Barboza posted 4 patches 5 years, 8 months ago
[PATCH v2 1/4] numa_conf.c: add helper functions for cpumap operations
Posted by Daniel Henrique Barboza 5 years, 8 months ago
These helpers will be used in an auto-fill feature for incomplete
NUMA topologies in the next patch.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 src/conf/numa_conf.c     | 46 ++++++++++++++++++++++++++++++++++++++++
 src/conf/numa_conf.h     |  3 +++
 src/libvirt_private.syms |  1 +
 3 files changed, 50 insertions(+)

diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index 09811cb51b..d022685284 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -1372,3 +1372,49 @@ virDomainNumaGetMemorySize(virDomainNumaPtr numa)
 
     return ret;
 }
+
+
+static int
+virDomainNumaRemoveCPUsFromMap(virBitmapPtr result, virBitmapPtr exclude)
+{
+    size_t i;
+
+    for (i = 0; i < virBitmapSize(exclude); i++) {
+        if (!virBitmapIsBitSet(exclude, i))
+            continue;
+
+        if (virBitmapClearBitExpand(result, i) < 0)
+            return -1;
+    }
+
+    return 0;
+}
+
+
+int
+virDomainNumaFillCPUsInNode(virDomainNumaPtr numa, size_t node,
+                            unsigned int maxCpus)
+{
+    g_autoptr(virBitmap) maxCPUsBitmap = virBitmapNew(maxCpus);
+    size_t i;
+
+    if (node >= virDomainNumaGetNodeCount(numa))
+        return -1;
+
+    virBitmapSetAll(maxCPUsBitmap);
+
+    for (i = 0; i < numa->nmem_nodes; i++) {
+        virBitmapPtr nodeCpus = virDomainNumaGetNodeCpumask(numa, i);
+
+        if (i == node)
+            continue;
+
+        if (virDomainNumaRemoveCPUsFromMap(maxCPUsBitmap, nodeCpus) < 0)
+            return -1;
+    }
+
+    virBitmapFree(numa->mem_nodes[node].cpumask);
+    numa->mem_nodes[node].cpumask = g_steal_pointer(&maxCPUsBitmap);
+
+    return 0;
+}
diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h
index ae3599bb8b..cdf87a87e8 100644
--- a/src/conf/numa_conf.h
+++ b/src/conf/numa_conf.h
@@ -185,3 +185,6 @@ int virDomainNumaDefCPUParseXML(virDomainNumaPtr def, xmlXPathContextPtr ctxt);
 int virDomainNumaDefCPUFormatXML(virBufferPtr buf, virDomainNumaPtr def);
 
 unsigned int virDomainNumaGetCPUCountTotal(virDomainNumaPtr numa);
+
+int virDomainNumaFillCPUsInNode(virDomainNumaPtr numa, size_t node,
+                                unsigned int maxCpus);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a6af44fe1c..788e08045a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -834,6 +834,7 @@ virDomainMemoryAccessTypeFromString;
 virDomainMemoryAccessTypeToString;
 virDomainNumaCheckABIStability;
 virDomainNumaEquals;
+virDomainNumaFillCPUsInNode;
 virDomainNumaFree;
 virDomainNumaGetCPUCountTotal;
 virDomainNumaGetMaxCPUID;
-- 
2.26.2

Re: [PATCH v2 1/4] numa_conf.c: add helper functions for cpumap operations
Posted by Michal Privoznik 5 years, 7 months ago
On 6/10/20 8:35 PM, Daniel Henrique Barboza wrote:
> These helpers will be used in an auto-fill feature for incomplete
> NUMA topologies in the next patch.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>   src/conf/numa_conf.c     | 46 ++++++++++++++++++++++++++++++++++++++++
>   src/conf/numa_conf.h     |  3 +++
>   src/libvirt_private.syms |  1 +
>   3 files changed, 50 insertions(+)
> 
> diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
> index 09811cb51b..d022685284 100644
> --- a/src/conf/numa_conf.c
> +++ b/src/conf/numa_conf.c
> @@ -1372,3 +1372,49 @@ virDomainNumaGetMemorySize(virDomainNumaPtr numa)
>   
>       return ret;
>   }
> +
> +
> +static int
> +virDomainNumaRemoveCPUsFromMap(virBitmapPtr result, virBitmapPtr exclude)
> +{
> +    size_t i;
> +
> +    for (i = 0; i < virBitmapSize(exclude); i++) {
> +        if (!virBitmapIsBitSet(exclude, i))
> +            continue;
> +
> +        if (virBitmapClearBitExpand(result, i) < 0)
> +            return -1;
> +    }
> +
> +    return 0;
> +}
> +

This exactly what virBitmapSubtract() does :-)

> +
> +int
> +virDomainNumaFillCPUsInNode(virDomainNumaPtr numa, size_t node,
> +                            unsigned int maxCpus)
> +{
> +    g_autoptr(virBitmap) maxCPUsBitmap = virBitmapNew(maxCpus);
> +    size_t i;
> +
> +    if (node >= virDomainNumaGetNodeCount(numa))
> +        return -1;
> +
> +    virBitmapSetAll(maxCPUsBitmap);
> +
> +    for (i = 0; i < numa->nmem_nodes; i++) {
> +        virBitmapPtr nodeCpus = virDomainNumaGetNodeCpumask(numa, i);
> +
> +        if (i == node)
> +            continue;
> +
> +        if (virDomainNumaRemoveCPUsFromMap(maxCPUsBitmap, nodeCpus) < 0)
> +            return -1;
> +    }
> +
> +    virBitmapFree(numa->mem_nodes[node].cpumask);
> +    numa->mem_nodes[node].cpumask = g_steal_pointer(&maxCPUsBitmap);

For some weird reason, I'd feel better if the bitmap is replaced only if 
it differs. I can't really explain why.


Michal

Re: [PATCH v2 1/4] numa_conf.c: add helper functions for cpumap operations
Posted by Daniel Henrique Barboza 5 years, 7 months ago

On 6/18/20 7:34 AM, Michal Privoznik wrote:
> On 6/10/20 8:35 PM, Daniel Henrique Barboza wrote:
>> These helpers will be used in an auto-fill feature for incomplete
>> NUMA topologies in the next patch.
>>
>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>> ---
>>   src/conf/numa_conf.c     | 46 ++++++++++++++++++++++++++++++++++++++++
>>   src/conf/numa_conf.h     |  3 +++
>>   src/libvirt_private.syms |  1 +
>>   3 files changed, 50 insertions(+)
>>
>> diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
>> index 09811cb51b..d022685284 100644
>> --- a/src/conf/numa_conf.c
>> +++ b/src/conf/numa_conf.c
>> @@ -1372,3 +1372,49 @@ virDomainNumaGetMemorySize(virDomainNumaPtr numa)
>>       return ret;
>>   }
>> +
>> +
>> +static int
>> +virDomainNumaRemoveCPUsFromMap(virBitmapPtr result, virBitmapPtr exclude)
>> +{
>> +    size_t i;
>> +
>> +    for (i = 0; i < virBitmapSize(exclude); i++) {
>> +        if (!virBitmapIsBitSet(exclude, i))
>> +            continue;
>> +
>> +        if (virBitmapClearBitExpand(result, i) < 0)
>> +            return -1;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
> 
> This exactly what virBitmapSubtract() does :-)

Completely forgot about virBitmapSubtract() after I tried to use it
in an earlier interaction of the code and it didn't suit what I
need. Granted, I was doing it a convoluted way of doing what I'm
doing now. virBitmapSubtract really works as a replacement in the
code below.


> 
>> +
>> +int
>> +virDomainNumaFillCPUsInNode(virDomainNumaPtr numa, size_t node,
>> +                            unsigned int maxCpus)
>> +{
>> +    g_autoptr(virBitmap) maxCPUsBitmap = virBitmapNew(maxCpus);
>> +    size_t i;
>> +
>> +    if (node >= virDomainNumaGetNodeCount(numa))
>> +        return -1;
>> +
>> +    virBitmapSetAll(maxCPUsBitmap);
>> +
>> +    for (i = 0; i < numa->nmem_nodes; i++) {
>> +        virBitmapPtr nodeCpus = virDomainNumaGetNodeCpumask(numa, i);
>> +
>> +        if (i == node)
>> +            continue;
>> +
>> +        if (virDomainNumaRemoveCPUsFromMap(maxCPUsBitmap, nodeCpus) < 0)
>> +            return -1;
>> +    }
>> +
>> +    virBitmapFree(numa->mem_nodes[node].cpumask);
>> +    numa->mem_nodes[node].cpumask = g_steal_pointer(&maxCPUsBitmap);
> 
> For some weird reason, I'd feel better if the bitmap is replaced only if it differs. I can't really explain why.

I think it's ok to check for virBitmapEqual() before replacing the
existing one.



DHB

> 
> 
> Michal
>