examples/power: allow use of more than 64 cores
vm_power_manager has up to now been limited to 64-cores because of the extensive use of uint64 type for bitmasks. This patch set converts those uint64s to character arrays, and increases the limit of cores from 64 to 256, and allowing users to increase this by increasing the #define.
First of all we convert all the relevant uint64_t's to char arrays. Then we remove the unneeded mask functions that were limited to 64 cores. Also extend the guest functionality and finally rause the number of supported cores to something more sensible, i.e. 256.
[1/4] examples/power: change 64-bit masks to arrays [2/4] examples/power: remove mask functions [3/4] examples/power: allow vms to use lcores over 63 [4/4] examples/power: increase MAX_CPUS to 256
PATCH 1/4 examples/power: change 64-bit masks to arrays
vm_power_manager currently makes use of uint64_t masks to keep track of cores in use, limiting use of the app to only being able to manage the first 64 cores in a multi-core system. Many modern systems have core counts greater than 64, so this limitation needs to be removed.
This patch converts the relevant 64-bit masks to character arrays.
--- examples/vm_power_manager/channel_manager.c | 111 +++++++++++--------- examples/vm_power_manager/channel_manager.h | 2 +- examples/vm_power_manager/vm_power_cli.c | 6 +- 3 files changed, 68 insertions(+), 51 deletions(-)
examples/vm_power_manager/channel_manager.c +64 -47
@@ -29,14 +29,11 @@#include "channel_manager.h" #include "channel_commands.h" #include "channel_monitor.h" +#include "power_manager.h" #define RTE_LOGTYPE_CHANNEL_MANAGER RTE_LOGTYPE_USER1 -#define ITERATIVE_BITMASK_CHECK_64 (mask_u64b, i) \ - for (i = 0; mask_u64b; mask_u64b &= ~(1ULL << i++)) \ - if ((mask_u64b >> i) & 1) \ -/* Global pointer to libvirt connection */ static virConnectPtr global_vir_conn_ptr;
@@ -54,7 +51,7 @@ struct virtual_machine_info {char name[CHANNEL_MGR_MAX_NAME_LEN ];rte_atomic64_t pcpu_mask[CHANNEL_CMDS_MAX_CPUS ];struct channel_info *channels[CHANNEL_CMDS_MAX_VM_CHANNELS ]; -uint64_t channel_mask; +char channel_mask[POWER_MGR_MAX_CPUS ];uint8_t num_channels;enum vm_status status;virDomainPtr domainPtr;
@@ -135,12 +132,14 @@ update_pcpus_mask(struct virtual_machine_info *vm_info) }int -set_pcpus_mask (char *vm_name,unsigned vcpu,uint64_t core_mask) +set_pcpus_mask (char *vm_name,unsigned int vcpu,char *core_mask) {unsigned i =0 ;int flags =VIR_DOMAIN_AFFECT_LIVE |VIR_DOMAIN_AFFECT_CONFIG ;struct virtual_machine_info *vm_info; -uint64_t mask = core_mask; +char mask[POWER_MGR_MAX_CPUS ]; + +memcpy (mask, core_mask,POWER_MGR_MAX_CPUS );if (vcpu >=CHANNEL_CMDS_MAX_CPUS ) { RTE_LOG(ERR, CHANNEL_MANAGER, "vCPU(%u) exceeds max allowable(%d)\n",
@@ -156,8 +155,8 @@ set_pcpus_mask(char *vm_name, unsigned vcpu, uint64_t core_mask)if (!virDomainIsActive (vm_info->domainPtr)) {RTE_LOG (ERR ,CHANNEL_MANAGER ,"Unable to set vCPU(%u) to pCPU " -"mask(0x%" PRIx64") for VM '%s', VM is not active\n" , - vcpu, core_mask, vm_info->name); +" for VM '%s', VM is not active\n" , + vcpu, vm_info->name);return -1 ; }
@@ -167,22 +166,25 @@ set_pcpus_mask(char *vm_name, unsigned vcpu, uint64_t core_mask)return -1 ; }memset (global_cpumaps,0 ,CHANNEL_CMDS_MAX_CPUS * global_maplen); - ITERATIVE_BITMASK_CHECK_64(mask, i) { -VIR_USE_CPU (global_cpumaps, i); -if (i >= global_n_host_cpus) { -RTE_LOG (ERR ,CHANNEL_MANAGER ,"CPU(%u) exceeds the available " -"number of CPUs(%u)\n" , i, global_n_host_cpus); -return -1 ; +for (i =0 ; i <POWER_MGR_MAX_CPUS ; i++) { +if (mask[i] ==1 ) { +VIR_USE_CPU (global_cpumaps, i); +if (i >= global_n_host_cpus) { +RTE_LOG (ERR ,CHANNEL_MANAGER ,"CPU(%u) exceeds the available " +"number of CPUs(%u)\n" , + i, global_n_host_cpus); +return -1 ; + } } }if (virDomainPinVcpuFlags (vm_info->domainPtr, vcpu, global_cpumaps, global_maplen, flags) <0 ) {RTE_LOG (ERR ,CHANNEL_MANAGER ,"Unable to set vCPU(%u) to pCPU " -"mask(0x%" PRIx64") for VM '%s'\n" , vcpu, core_mask, +" for VM '%s'\n" , vcpu, vm_info->name);return -1 ; } -rte_atomic64_set (&vm_info->pcpu_mask[vcpu], core_mask); +memcpy (&vm_info->pcpu_mask[vcpu], core_mask,POWER_MGR_MAX_CPUS );return 0 ; }
@@ -190,7 +192,11 @@ set_pcpus_mask(char *vm_name, unsigned vcpu, uint64_t core_mask)int set_pcpu (char *vm_name,unsigned vcpu,unsigned core_num) { -uint64_t mask =1ULL << core_num; +char mask[POWER_MGR_MAX_CPUS ]; + +memset (mask,0 ,POWER_MGR_MAX_CPUS ); + + mask[core_num] =1 ;return set_pcpus_mask (vm_name, vcpu, mask); }
@@ -211,7 +217,7 @@ static inline int channel_exists(struct virtual_machine_info *vm_info, unsigned channel_num) {rte_spinlock_lock (&(vm_info->config_spinlock)); -if (vm_info->channel_mask & (1ULL << channel_num)) { +if (vm_info->channel_mask[channel_num]) {rte_spinlock_unlock (&(vm_info->config_spinlock));return 1 ; }
@@ -343,7 +349,7 @@ setup_channel_info(struct virtual_machine_info **vm_info_dptr, }rte_spinlock_lock (&(vm_info->config_spinlock)); vm_info->num_channels++; - vm_info->channel_mask |=1ULL << channel_num; + vm_info->channel_mask[channel_num] =1 ; vm_info->channels[channel_num] = chan_info; chan_info->status =CHANNEL_MGR_CHANNEL_CONNECTED ;rte_spinlock_unlock (&(vm_info->config_spinlock));
@@ -590,7 +596,7 @@ remove_channel(struct channel_info **chan_info_dptr) vm_info = (struct virtual_machine_info *)chan_info->priv_info;rte_spinlock_lock (&(vm_info->config_spinlock)); - vm_info->channel_mask &= ~(1ULL << chan_info->channel_num); + vm_info->channel_mask[chan_info->channel_num] =0 ; vm_info->num_channels--;rte_spinlock_unlock (&(vm_info->config_spinlock));
@@ -603,7 +609,7 @@ set_channel_status_all(const char *vm_name, enum channel_status status) {struct virtual_machine_info *vm_info;unsigned i; -uint64_t mask; +char mask[POWER_MGR_MAX_CPUS ];int num_channels_changed =0 ; if (!(status == CHANNEL_MGR_CHANNEL_CONNECTED ||
@@ -619,10 +625,12 @@ set_channel_status_all(const char *vm_name, enum channel_status status) }rte_spinlock_lock (&(vm_info->config_spinlock)); - mask = vm_info->channel_mask; - ITERATIVE_BITMASK_CHECK_64(mask, i) { - vm_info->channels[i]->status = status; - num_channels_changed++; +memcpy (mask, (char *)vm_info->channel_mask,POWER_MGR_MAX_CPUS ); +for (i =0 ; i <POWER_MGR_MAX_CPUS ; i++) { +if (mask[i] ==1 ) { + vm_info->channels[i]->status = status; + num_channels_changed++; + } }rte_spinlock_unlock (&(vm_info->config_spinlock));return num_channels_changed;
@@ -665,7 +673,7 @@ get_all_vm(int *num_vm, int *num_vcpu)virNodeInfo node_info;virDomainPtr *domptr; -uint64_t mask; +char mask[POWER_MGR_MAX_CPUS ];int i, ii, numVcpus[MAX_VCPUS ], cpu, n_vcpus;unsigned int jj;const char *vm_name;
@@ -714,15 +722,16 @@ get_all_vm(int *num_vm, int *num_vcpu)/* Save pcpu in use by libvirt VMs */ for (ii =0 ; ii < n_vcpus; ii++) { - mask =0 ; +memset (mask,0 ,POWER_MGR_MAX_CPUS );for (jj =0 ; jj < global_n_host_cpus; jj++) {if (VIR_CPU_USABLE (global_cpumaps, global_maplen, ii, jj) >0 ) { - mask |=1ULL << jj; + mask[jj] =1 ; } } - ITERATIVE_BITMASK_CHECK_64(mask, cpu) { - lvm_info[i].pcpus[ii] = cpu; +for (cpu =0 ; cpu <POWER_MGR_MAX_CPUS ; cpu++) { +if (mask[cpu]) + lvm_info[i].pcpus[ii] = cpu; } } }
@@ -733,7 +742,7 @@ get_info_vm(const char *vm_name, struct vm_info *info) {struct virtual_machine_info *vm_info;unsigned i, channel_num =0 ; -uint64_t mask; +char mask[POWER_MGR_MAX_CPUS ]; vm_info =find_domain_by_name (vm_name);if (vm_info ==NULL ) {
@@ -746,14 +755,19 @@ get_info_vm(const char *vm_name, struct vm_info *info)rte_spinlock_lock (&(vm_info->config_spinlock)); - mask = vm_info->channel_mask; - ITERATIVE_BITMASK_CHECK_64(mask, i) { - info->channels[channel_num].channel_num = i; -memcpy (info->channels[channel_num].channel_path, - vm_info->channels[i]->channel_path,UNIX_PATH_MAX ); - info->channels[channel_num].status = vm_info->channels[i]->status; - info->channels[channel_num].fd = vm_info->channels[i]->fd; - channel_num++; +memcpy (mask, (char *)vm_info->channel_mask,POWER_MGR_MAX_CPUS ); +for (i =0 ; i <POWER_MGR_MAX_CPUS ; i++) { +if (mask[i] ==1 ) { + info->channels[channel_num].channel_num = i; +memcpy (info->channels[channel_num].channel_path, + vm_info->channels[i]->channel_path, +UNIX_PATH_MAX ); + info->channels[channel_num].status = + vm_info->channels[i]->status; + info->channels[channel_num].fd = + vm_info->channels[i]->fd; + channel_num++; + } } info->num_channels = channel_num;
@@ -822,7 +836,7 @@ add_vm(const char *vm_name) }strncpy (new_domain->name, vm_name,sizeof (new_domain->name)); new_domain->name[sizeof (new_domain->name) -1 ] ='\0' ; - new_domain->channel_mask =0 ; +memset (new_domain->channel_mask,0 ,POWER_MGR_MAX_CPUS ); new_domain->num_channels =0 ;if (!virDomainIsActive (dom_ptr))
@@ -940,18 +954,21 @@ voidchannel_manager_exit (void) {unsigned i; -uint64_t mask; +char mask[POWER_MGR_MAX_CPUS ];struct virtual_machine_info *vm_info; LIST_FOREACH(vm_info, &vm_list_head, vms_info) {rte_spinlock_lock (&(vm_info->config_spinlock)); - mask = vm_info->channel_mask; - ITERATIVE_BITMASK_CHECK_64(mask, i) { -remove_channel_from_monitor (vm_info->channels[i]); -close (vm_info->channels[i]->fd); -rte_free (vm_info->channels[i]); +memcpy (mask, (char *)vm_info->channel_mask,POWER_MGR_MAX_CPUS ); +for (i =0 ; i <POWER_MGR_MAX_CPUS ; i++) { +if (mask[i] ==1 ) { +remove_channel_from_monitor ( + vm_info->channels[i]); +close (vm_info->channels[i]->fd); +rte_free (vm_info->channels[i]); + } }rte_spinlock_unlock (&(vm_info->config_spinlock));
examples/vm_power_manager/channel_manager.h +1 -1
@@ -149,7 +149,7 @@ uint64_t get_pcpus_mask(struct channel_info *chan_info, unsigned vcpu);* - 0 on success. * - Negative on error. */ -int set_pcpus_mask (char *vm_name,unsigned vcpu,uint64_t core_mask); +int set_pcpus_mask (char *vm_name,unsigned int vcpu,char *core_mask);/** * Set the Physical CPU for the specified vCPU.
examples/vm_power_manager/vm_power_cli.c +3 -3
@@ -128,7 +128,7 @@ struct cmd_set_pcpu_mask_result {cmdline_fixed_string_t set_pcpu_mask;cmdline_fixed_string_t vm_name;uint8_t vcpu; -uint64_t core_mask; +char core_mask[POWER_MGR_MAX_CPUS ]; }; static void
@@ -139,10 +139,10 @@ cmd_set_pcpu_mask_parsed(void *parsed_result, struct cmdline *cl,if (set_pcpus_mask (res->vm_name, res->vcpu, res->core_mask) ==0 )cmdline_printf (cl,"Pinned vCPU(%" PRId8") to pCPU core " -"mask(0x%" PRIx64")\n" , res->vcpu, res->core_mask); +"\n" , res->vcpu);else cmdline_printf (cl,"Unable to pin vCPU(%" PRId8") to pCPU core " -"mask(0x%" PRIx64")\n" , res->vcpu, res->core_mask); +"\n" , res->vcpu); } cmdline_parse_token_string_t cmd_set_pcpu_mask =
Signature · 1 line
2.17.1
Re: [PATCH v1 1/4] examples/power: change 64-bit masks to arrays
Quoted 8 lines · David Hunt
vm_power_managercurrently makes use ofuint64_tmasks to keep track of cores in use, limiting use of the app to only being able to manage the first 64 cores in a multi-core system. Many modern systems have core counts greater than 64, so this limitation needs to be removed.This patch converts the relevant 64-bit masks to character arrays.
Signed-off-by David Hunt <[email protected]>
Hi Dave,
Overall looks OK, but some comments below.
Quoted 65 lines
--- examples/vm_power_manager/channel_manager.c | 111 +++++++++++--------- examples/vm_power_manager/channel_manager.h | 2 +- examples/vm_power_manager/vm_power_cli.c | 6 +- 3 files changed, 68 insertions(+), 51 deletions(-)examples/vm_power_manager/channel_manager.c +8
-9@@ -29,14 +29,11 @@#include "channel_manager.h" #include "channel_commands.h" #include "channel_monitor.h" +#include "power_manager.h" #define RTE_LOGTYPE_CHANNEL_MANAGER RTE_LOGTYPE_USER1 -#define ITERATIVE_BITMASK_CHECK_64 (mask_u64b, i) \ - for (i = 0; mask_u64b; mask_u64b &= ~(1ULL << i++)) \ - if ((mask_u64b >> i) & 1) \ -/* Global pointer to libvirt connection */ static virConnectPtr global_vir_conn_ptr;@@ -54,7 +51,7 @@ struct virtual_machine_info {char name[CHANNEL_MGR_MAX_NAME_LEN ];rte_atomic64_t pcpu_mask[CHANNEL_CMDS_MAX_CPUS ];struct channel_info *channels[CHANNEL_CMDS_MAX_VM_CHANNELS ]; -uint64_t channel_mask; +char channel_mask[POWER_MGR_MAX_CPUS ];uint8_t num_channels;enum vm_status status;virDomainPtr domainPtr;@@ -135,12 +132,14 @@ update_pcpus_mask(struct virtual_machine_info *vm_info) }int -set_pcpus_mask (char *vm_name,unsigned vcpu,uint64_t core_mask) +set_pcpus_mask (char *vm_name,unsigned int vcpu,char *core_mask) {unsigned i =0 ;int flags =VIR_DOMAIN_AFFECT_LIVE |VIR_DOMAIN_AFFECT_CONFIG ;struct virtual_machine_info *vm_info; -uint64_t mask = core_mask; +char mask[POWER_MGR_MAX_CPUS ]; + +memcpy (mask, core_mask,POWER_MGR_MAX_CPUS );if (vcpu >=CHANNEL_CMDS_MAX_CPUS ) { RTE_LOG(ERR, CHANNEL_MANAGER, "vCPU(%u) exceeds max allowable(%d)\n",@@ -156,8 +155,8 @@ set_pcpus_mask(char *vm_name, unsigned vcpu, uint64_t core_mask)if (!virDomainIsActive (vm_info->domainPtr)) {RTE_LOG (ERR ,CHANNEL_MANAGER ,"Unable to set vCPU(%u) to pCPU " -"mask(0x%" PRIx64") for VM '%s', VM is not active\n" , - vcpu, core_mask, vm_info->name); +" for VM '%s', VM is not active\n" , + vcpu, vm_info->name);return -1 ; }@@ -167,22 +166,25 @@ set_pcpus_mask(char *vm_name, unsigned vcpu, uint64_t core_mask)return -1 ;
examples/vm_power_manager/channel_manager.c set_pcpus_mask(char *vm_name, unsigned vcpu, uint64_t core_mask) · line 170
Quoted 10 lines
examples/vm_power_manager/channel_manager.c +2
-6}memset (global_cpumaps,0 ,CHANNEL_CMDS_MAX_CPUS * global_maplen); - ITERATIVE_BITMASK_CHECK_64(mask, i) { -VIR_USE_CPU (global_cpumaps, i); -if (i >= global_n_host_cpus) { -RTE_LOG (ERR ,CHANNEL_MANAGER ,"CPU(%u) exceeds the available " -"number of CPUs(%u)\n" , i, global_n_host_cpus); -return -1 ; +for (i =0 ; i <POWER_MGR_MAX_CPUS ; i++) { +if (mask[i] ==1 ) {
Here and in other places - early exit would've avoided unneeded extra indents, e.g.
if (mask[i] != 1)
continue;
<do stuff>
Quoted 9 lines
examples/vm_power_manager/channel_manager.c +7
-0+VIR_USE_CPU (global_cpumaps, i); +if (i >= global_n_host_cpus) { +RTE_LOG (ERR ,CHANNEL_MANAGER ,"CPU(%u) exceeds the available " +"number of CPUs(%u)\n" , + i, global_n_host_cpus); +return -1 ; + } } }
examples/vm_power_manager/channel_manager.c set_pcpus_mask(char *vm_name, unsigned vcpu, uint64_t core_mask) · line 187
Quoted 10 lines
examples/vm_power_manager/channel_manager.c +2
-2if (virDomainPinVcpuFlags (vm_info->domainPtr, vcpu, global_cpumaps, global_maplen, flags) <0 ) {RTE_LOG (ERR ,CHANNEL_MANAGER ,"Unable to set vCPU(%u) to pCPU " -"mask(0x%" PRIx64") for VM '%s'\n" , vcpu, core_mask, +" for VM '%s'\n" , vcpu, vm_info->name);return -1 ; } -rte_atomic64_set (&vm_info->pcpu_mask[vcpu], core_mask); +memcpy (&vm_info->pcpu_mask[vcpu], core_mask,POWER_MGR_MAX_CPUS );
I'm probably missing something here, but at face value, replacing an atomic set operation with a simple memcpy is not equivalent. Is there any locking that's missing from here? Or was it that atomics were not necessary in the first place?
Quoted 5 lines
examples/vm_power_manager/channel_manager.c +0
-0return 0 ; }@@ -190,7 +192,11 @@ set_pcpus_mask(char *vm_name, unsigned vcpu, uint64_t core_mask)int
examples/vm_power_manager/channel_manager.c set_pcpus_mask(char *vm_name, unsigned vcpu, uint64_t core_mask) · line 201
Quoted 10 lines
examples/vm_power_manager/channel_manager.c +5
-1set_pcpu(char *vm_name, unsigned vcpu, unsigned core_num) { -uint64_t mask =1ULL << core_num; +char mask[POWER_MGR_MAX_CPUS ]; + +memset (mask,0 ,POWER_MGR_MAX_CPUS ); + + mask[core_num] =1 ;return set_pcpus_mask (vm_name, vcpu, mask);
This was a thin wrapper to get around dealing with masks. Now that you're dealing with array indexes, this function seems redundant (and creates an extra mask/memset in the process).
examples/vm_power_manager/channel_manager.c static inline int · line 220
Quoted 7 lines
examples/vm_power_manager/channel_manager.c +1
-1}@@ -211,7 +217,7 @@ static inline int channel_exists(struct virtual_machine_info *vm_info, unsigned channel_num) {rte_spinlock_lock (&(vm_info->config_spinlock)); -if (vm_info->channel_mask & (1ULL << channel_num)) { +if (vm_info->channel_mask[channel_num]) {
You seem to be using (val) and (val == 1) interchangeably. This worked for masks, but will not work for char arrays. Comparisons for values should be explicit in this and other similar cases.
Quoted 34 lines
examples/vm_power_manager/channel_manager.c +3
-3rte_spinlock_unlock (&(vm_info->config_spinlock));return 1 ; }@@ -343,7 +349,7 @@ setup_channel_info(struct virtual_machine_info **vm_info_dptr, }rte_spinlock_lock (&(vm_info->config_spinlock)); vm_info->num_channels++; - vm_info->channel_mask |=1ULL << channel_num; + vm_info->channel_mask[channel_num] =1 ; vm_info->channels[channel_num] = chan_info; chan_info->status =CHANNEL_MGR_CHANNEL_CONNECTED ;rte_spinlock_unlock (&(vm_info->config_spinlock));@@ -590,7 +596,7 @@ remove_channel(struct channel_info **chan_info_dptr) vm_info = (struct virtual_machine_info *)chan_info->priv_info;rte_spinlock_lock (&(vm_info->config_spinlock)); - vm_info->channel_mask &= ~(1ULL << chan_info->channel_num); + vm_info->channel_mask[chan_info->channel_num] =0 ; vm_info->num_channels--;rte_spinlock_unlock (&(vm_info->config_spinlock));@@ -603,7 +609,7 @@ set_channel_status_all(const char *vm_name, enum channel_status status) {struct virtual_machine_info *vm_info;unsigned i; -uint64_t mask; +char mask[POWER_MGR_MAX_CPUS ];int num_channels_changed =0 ; if (!(status == CHANNEL_MGR_CHANNEL_CONNECTED ||@@ -619,10 +625,12 @@ set_channel_status_all(const char *vm_name, enum channel_status status) }rte_spinlock_lock (&(vm_info->config_spinlock));
examples/vm_power_manager/channel_manager.c set_channel_status_all(const char *vm_name, enum channel_status status) · line 633
Quoted 10 lines
examples/vm_power_manager/channel_manager.c +6
-4- mask = vm_info->channel_mask; - ITERATIVE_BITMASK_CHECK_64(mask, i) { - vm_info->channels[i]->status = status; - num_channels_changed++; +memcpy (mask, (char *)vm_info->channel_mask,POWER_MGR_MAX_CPUS ); +for (i =0 ; i <POWER_MGR_MAX_CPUS ; i++) { +if (mask[i] ==1 ) { + vm_info->channels[i]->status = status; + num_channels_changed++; + }
Same as above re: indent - perhaps, early exit (well, early continue...) would make things easier to read.
Quoted 20 lines
examples/vm_power_manager/channel_manager.c +2
-2}rte_spinlock_unlock (&(vm_info->config_spinlock));return num_channels_changed;@@ -665,7 +673,7 @@ get_all_vm(int *num_vm, int *num_vcpu)virNodeInfo node_info;virDomainPtr *domptr; -uint64_t mask; +char mask[POWER_MGR_MAX_CPUS ];int i, ii, numVcpus[MAX_VCPUS ], cpu, n_vcpus;unsigned int jj;const char *vm_name;@@ -714,15 +722,16 @@ get_all_vm(int *num_vm, int *num_vcpu)/* Save pcpu in use by libvirt VMs */ for (ii =0 ; ii < n_vcpus; ii++) { - mask =0 ; +memset (mask,0 ,POWER_MGR_MAX_CPUS );for (jj =0 ; jj < global_n_host_cpus; jj++) { if (VIR_CPU_USABLE(global_cpumaps,
examples/vm_power_manager/channel_manager.c get_all_vm(int *num_vm, int *num_vcpu) · line 734
Quoted 10 lines
examples/vm_power_manager/channel_manager.c +4
-3global_maplen, ii, jj) > 0) { - mask |=1ULL << jj; + mask[jj] =1 ; } } - ITERATIVE_BITMASK_CHECK_64(mask, cpu) { - lvm_info[i].pcpus[ii] = cpu; +for (cpu =0 ; cpu <POWER_MGR_MAX_CPUS ; cpu++) { +if (mask[cpu]) + lvm_info[i].pcpus[ii] = cpu;
Same, should be mask[cpu] == 1.
Also, are two loops necessary?
Quoted 17 lines
examples/vm_power_manager/channel_manager.c +1
-2} } }@@ -733,7 +742,7 @@ get_info_vm(const char *vm_name, struct vm_info *info) {struct virtual_machine_info *vm_info;unsigned i, channel_num =0 ; -uint64_t mask; +char mask[POWER_MGR_MAX_CPUS ]; vm_info =find_domain_by_name (vm_name);if (vm_info ==NULL ) {@@ -746,14 +755,19 @@ get_info_vm(const char *vm_name, struct vm_info *info)rte_spinlock_lock (&(vm_info->config_spinlock)); - mask = vm_info->channel_mask;
examples/vm_power_manager/channel_manager.c get_info_vm(const char *vm_name, struct vm_info *info) · line 760
Quoted 10 lines
examples/vm_power_manager/channel_manager.c +3
-7- ITERATIVE_BITMASK_CHECK_64(mask, i) { - info->channels[channel_num].channel_num = i; -memcpy (info->channels[channel_num].channel_path, - vm_info->channels[i]->channel_path,UNIX_PATH_MAX ); - info->channels[channel_num].status = vm_info->channels[i]->status; - info->channels[channel_num].fd = vm_info->channels[i]->fd; - channel_num++; +memcpy (mask, (char *)vm_info->channel_mask,POWER_MGR_MAX_CPUS ); +for (i =0 ; i <POWER_MGR_MAX_CPUS ; i++) { +if (mask[i] ==1 ) {
Same - early exit would make things more readable.
Quoted 32 lines
examples/vm_power_manager/channel_manager.c +12
-2+ info->channels[channel_num].channel_num = i; +memcpy (info->channels[channel_num].channel_path, + vm_info->channels[i]->channel_path, +UNIX_PATH_MAX ); + info->channels[channel_num].status = + vm_info->channels[i]->status; + info->channels[channel_num].fd = + vm_info->channels[i]->fd; + channel_num++; + } } info->num_channels = channel_num;@@ -822,7 +836,7 @@ add_vm(const char *vm_name) }strncpy (new_domain->name, vm_name,sizeof (new_domain->name)); new_domain->name[sizeof (new_domain->name) -1 ] ='\0' ; - new_domain->channel_mask =0 ; +memset (new_domain->channel_mask,0 ,POWER_MGR_MAX_CPUS ); new_domain->num_channels =0 ;if (!virDomainIsActive (dom_ptr))@@ -940,18 +954,21 @@ voidchannel_manager_exit (void) {unsigned i; -uint64_t mask; +char mask[POWER_MGR_MAX_CPUS ];struct virtual_machine_info *vm_info; LIST_FOREACH(vm_info, &vm_list_head, vms_info) {
examples/vm_power_manager/channel_manager.c void · line 966
Quoted 10 lines
examples/vm_power_manager/channel_manager.c +3
-5rte_spinlock_lock (&(vm_info->config_spinlock)); - mask = vm_info->channel_mask; - ITERATIVE_BITMASK_CHECK_64(mask, i) { -remove_channel_from_monitor (vm_info->channels[i]); -close (vm_info->channels[i]->fd); -rte_free (vm_info->channels[i]); +memcpy (mask, (char *)vm_info->channel_mask,POWER_MGR_MAX_CPUS ); +for (i =0 ; i <POWER_MGR_MAX_CPUS ; i++) { +if (mask[i] ==1 ) {
Same - early exit.
Quoted 48 lines
examples/vm_power_manager/channel_manager.c +5
-0+remove_channel_from_monitor ( + vm_info->channels[i]); +close (vm_info->channels[i]->fd); +rte_free (vm_info->channels[i]); + } }rte_spinlock_unlock (&(vm_info->config_spinlock));examples/vm_power_manager/channel_manager.h +1
-1@@ -149,7 +149,7 @@ uint64_t get_pcpus_mask(struct channel_info *chan_info, unsigned vcpu);* - 0 on success. * - Negative on error. */ -int set_pcpus_mask (char *vm_name,unsigned vcpu,uint64_t core_mask); +int set_pcpus_mask (char *vm_name,unsigned int vcpu,char *core_mask);/** * Set the Physical CPU for the specified vCPU. examples/vm_power_manager/vm_power_cli.c +3
-3@@ -128,7 +128,7 @@ struct cmd_set_pcpu_mask_result {cmdline_fixed_string_t set_pcpu_mask;cmdline_fixed_string_t vm_name;uint8_t vcpu; -uint64_t core_mask; +char core_mask[POWER_MGR_MAX_CPUS ]; }; static void@@ -139,10 +139,10 @@ cmd_set_pcpu_mask_parsed(void *parsed_result, struct cmdline *cl,if (set_pcpus_mask (res->vm_name, res->vcpu, res->core_mask) ==0 )cmdline_printf (cl,"Pinned vCPU(%" PRId8") to pCPU core " -"mask(0x%" PRIx64")\n" , res->vcpu, res->core_mask); +"\n" , res->vcpu);else cmdline_printf (cl,"Unable to pin vCPU(%" PRId8") to pCPU core " -"mask(0x%" PRIx64")\n" , res->vcpu, res->core_mask); +"\n" , res->vcpu); } cmdline_parse_token_string_t cmd_set_pcpu_mask =
Signature · 2 lines
Thanks, Anatoly
Re: [PATCH v1 1/4] examples/power: change 64-bit masks to arrays
Hi Anatoly,
Quoted 87 lines · Anatoly
Quoted 8 lines · David Hunt
vm_power_managercurrently makes use ofuint64_tmasks to keep track of cores in use, limiting use of the app to only being able to manage the first 64 cores in a multi-core system. Many modern systems have core counts greater than 64, so this limitation needs to be removed.This patch converts the relevant 64-bit masks to character arrays.
Signed-off-by David Hunt <[email protected]>Hi Dave,
Overall looks OK, but some comments below.
Quoted 62 lines
---
examples/vm_power_manager/channel_manager.c| 111 +++++++++++---------examples/vm_power_manager/channel_manager.h| 2 +- examples/vm_power_manager/vm_power_cli.c | 6 +-3 files changed, 68 insertions(+), 51 deletions(-)examples/vm_power_manager/channel_manager.c +1
-3@@ -29,14 +29,11 @@ #include "channel_manager.h" #include "channel_commands.h" #include "channel_monitor.h" +#include "power_manager.h" #define RTE_LOGTYPE_CHANNEL_MANAGER RTE_LOGTYPE_USER1 -#define ITERATIVE_BITMASK_CHECK_64(mask_u64b, i) \ - for (i = 0; mask_u64b; mask_u64b &= ~(1ULL << i++)) \ - if ((mask_u64b >> i) & 1) \ -/* Global pointer to libvirt connection */ static virConnectPtr global_vir_conn_ptr; @@ -54,7 +51,7 @@ struct virtual_machine_info { char name[CHANNEL_MGR_MAX_NAME_LEN]; rte_atomic64_t pcpu_mask[CHANNEL_CMDS_MAX_CPUS];struct
channel_info*channels[CHANNEL_CMDS_MAX_VM_CHANNELS]; -uint64_tchannel_mask;
+ char channel_mask[POWER_MGR_MAX_CPUS];
uint8_tnum_channels;
enumvm_statusstatus;
virDomainPtr domainPtr;examples/vm_power_manager/channel_manager.c +0
-0@@ -135,12 +132,14 @@ update_pcpus_mask(struct virtual_machine_info *vm_info) }int
examples/vm_power_manager/channel_manager.c +1
-1-set_pcpus_mask(char *vm_name, unsigned vcpu, uint64_t core_mask) +set_pcpus_mask(char *vm_name, unsigned int vcpu, char *core_mask){
unsigned i = 0;
int flags = VIR_DOMAIN_AFFECT_LIVE|VIR_DOMAIN_AFFECT_CONFIG; structvirtual_machine_info*vm_info;
-uint64_tmask =core_mask;
+ char mask[POWER_MGR_MAX_CPUS];
+
+memcpy(mask, core_mask, POWER_MGR_MAX_CPUS);
if (vcpu >=CHANNEL_CMDS_MAX_CPUS) {
RTE_LOG(ERR,CHANNEL_MANAGER, "vCPU(%u) exceeds max allowable(%d)\n",examples/vm_power_manager/channel_manager.c +0
-0@@ -156,8 +155,8 @@ set_pcpus_mask(char *vm_name, unsigned vcpu, uint64_t core_mask) if (!virDomainIsActive(vm_info->domainPtr)) {RTE_LOG(ERR,
CHANNEL_MANAGER, "Unable to set vCPU(%u) to pCPU " - "mask(0x%"PRIx64")for VM '%s', VM is not active\n",
- vcpu,core_mask,vm_info->name);
+ " for VM '%s', VM is not active\n",
+ vcpu,vm_info->name);
return -1;
}
@@ -167,22 +166,25 @@set_pcpus_mask(char *vm_name, unsigned vcpu, uint64_t core_mask)return -1;
}
memset(global_cpumaps, 0 , CHANNEL_CMDS_MAX_CPUS * global_maplen);
-ITERATIVE_BITMASK_CHECK_64(mask, i){Quoted 8 lines
-
VIR_USE_CPU(global_cpumaps, i);
- if (i >=global_n_host_cpus) {
- RTE_LOG(ERR,CHANNEL_MANAGER, "CPU(%u) exceeds the available "
- "number of CPUs(%u)\n", i,global_n_host_cpus); - return -1;
+ for (i = 0; i <POWER_MGR_MAX_CPUS; i++) {
+ if (mask[i] == 1) {Here and in other places - early exit would've avoided unneeded extra indents, e.g.
Quoted 4 lines
if (mask[i] != 1)
continue;
<do stuff>
Sure, will do.
Quoted 21 lines
Quoted 12 lines
+
VIR_USE_CPU(global_cpumaps, i);
+ if (i >=global_n_host_cpus) {
+ RTE_LOG(ERR,CHANNEL_MANAGER, "CPU(%u) exceeds the available "
+ "number of CPUs(%u)\n",
+ i,global_n_host_cpus);
+ return -1;
+ }
}
}
if (virDomainPinVcpuFlags(vm_info->domainPtr, vcpu, global_cpumaps, global_maplen, flags)< 0) {Quoted 8 lines
RTE_LOG(ERR,
CHANNEL_MANAGER, "Unable to set vCPU(%u) to pCPU " - "mask(0x%"PRIx64")for VM '%s'\n", vcpu,core_mask, + " for VM '%s'\n", vcpu,
vm_info->name);
return -1;
}
-rte_atomic64_set(&vm_info->pcpu_mask[vcpu], core_mask); +memcpy(&vm_info->pcpu_mask[vcpu], core_mask, POWER_MGR_MAX_CPUS);
Quoted 1 line
I'm probably missing something here, but at face value, replacing an atomic set operation with a simple memcpy is not equivalent. Is there any locking that's missing from here? Or was it that atomics were not necessary in the first place?
Yes. The rest of the updates to the config space are protected by a spinlock, this case used an atomic because it was a uint64, so now that it's an array I've added a spinlock lock/unlock around the memcpy.
Quoted 18 lines
Quoted 14 lines
return 0;
}examples/vm_power_manager/channel_manager.c +0
-0@@ -190,7 +192,11 @@ set_pcpus_mask(char *vm_name, unsigned vcpu, uint64_t core_mask)int
set_pcpu(char *vm_name, unsigned vcpu, unsigned core_num){
-uint64_tmask = 1ULL <<core_num;
+ char mask[POWER_MGR_MAX_CPUS];examples/vm_power_manager/channel_manager.c +4
-0+ + memset(mask, 0, POWER_MGR_MAX_CPUS); + + mask[core_num] = 1;Quoted 1 line
return
set_pcpus_mask(vm_name, vcpu, mask);This was a thin wrapper to get around dealing with masks. Now that you're dealing with array indexes, this function seems redundant (and creates an extra mask/memset in the process).
Both functions are called from other files. So still needed, unfortunately. It may be possible to remove later in the series, if those calls are removed.
Quoted 9 lines
examples/vm_power_manager/channel_manager.c static inline int · line 220
Quoted 7 lines
}
examples/vm_power_manager/channel_manager.c +1
-1@@ -211,7 +217,7 @@ static inline int channel_exists(struct virtual_machine_info *vm_info, unsigned channel_num) { rte_spinlock_lock(&(vm_info->config_spinlock)); - if (vm_info->channel_mask & (1ULL << channel_num)) { + if (vm_info->channel_mask[channel_num]) {You seem to be using (val) and (val == 1) interchangeably. This worked for masks, but will not work for char arrays. Comparisons for values should be explicit in this and other similar cases.
Fixed.
Quoted 37 lines
Quoted 37 lines
examples/vm_power_manager/channel_manager.c +1
-1rte_spinlock_unlock (&(vm_info->config_spinlock)); return 1; }@@ -343,7 +349,7 @@ setup_channel_info(struct virtual_machine_info **vm_info_dptr, } rte_spinlock_lock(&(vm_info->config_spinlock)); vm_info->num_channels++; - vm_info->channel_mask |= 1ULL << channel_num; + vm_info->channel_mask[channel_num] = 1; vm_info->channels[channel_num] = chan_info; chan_info->status = CHANNEL_MGR_CHANNEL_CONNECTED;rte_spinlock_unlock(&(vm_info->config_spinlock));
examples/vm_power_manager/channel_manager.c +1
-1@@ -590,7 +596,7 @@ remove_channel(struct channel_info **chan_info_dptr) vm_info = (struct virtual_machine_info *)chan_info->priv_info; rte_spinlock_lock(&(vm_info->config_spinlock)); - vm_info->channel_mask &= ~(1ULL << chan_info->channel_num); + vm_info->channel_mask[chan_info->channel_num] = 0; vm_info->num_channels--; rte_spinlock_unlock(&(vm_info->config_spinlock)); @@ -603,7 +609,7 @@ set_channel_status_all(const char *vm_name, enum channel_status status) {struct
virtual_machine_info*vm_info; unsigned i;
-uint64_tmask;
+ char mask[POWER_MGR_MAX_CPUS];
intnum_channels_changed= 0;
if (!(status ==CHANNEL_MGR_CHANNEL_CONNECTED||examples/vm_power_manager/channel_manager.c +0
-4@@ -619,10 +625,12 @@ set_channel_status_all(const char *vm_name, enum channel_status status) } rte_spinlock_lock(&(vm_info->config_spinlock)); - mask = vm_info->channel_mask; - ITERATIVE_BITMASK_CHECK_64(mask, i) { - vm_info->channels[i]->status = status; - num_channels_changed++;
Quoted 8 lines
examples/vm_power_manager/channel_manager.c set_channel_status_all(const char *vm_name, enum channel_status status) · line 633
Quoted 6 lines
examples/vm_power_manager/channel_manager.c +6
-0+ memcpy(mask, (char *)vm_info->channel_mask, POWER_MGR_MAX_CPUS); + for (i = 0; i < POWER_MGR_MAX_CPUS; i++) { + if (mask[i] == 1) { + vm_info->channels[i]->status = status; + num_channels_changed++; + }Same as above re: indent - perhaps, early exit (well, early continue...) would make things easier to read.
Done.
Quoted 32 lines
Quoted 19 lines
examples/vm_power_manager/channel_manager.c +2
-2} rte_spinlock_unlock(&(vm_info->config_spinlock)); return num_channels_changed;@@ -665,7 +673,7 @@ get_all_vm(int *num_vm, int *num_vcpu) virNodeInfo node_info; virDomainPtr *domptr; - uint64_t mask; + char mask[POWER_MGR_MAX_CPUS]; int i, ii, numVcpus[MAX_VCPUS], cpu, n_vcpus; unsigned int jj; const char *vm_name;@@ -714,15 +722,16 @@ get_all_vm(int *num_vm, int *num_vcpu) /* Save pcpu in use by libvirt VMs */ for (ii = 0; ii < n_vcpus; ii++) { - mask = 0; + memset(mask, 0, POWER_MGR_MAX_CPUS); for (jj = 0; jj < global_n_host_cpus; jj++) { if (VIR_CPU_USABLE(global_cpumaps,examples/vm_power_manager/channel_manager.c get_all_vm(int *num_vm, int *num_vcpu) · line 733
Quoted 10 lines
examples/vm_power_manager/channel_manager.c +4
-3global_maplen, ii, jj) > 0) { - mask |= 1ULL << jj; + mask[jj] = 1; } } - ITERATIVE_BITMASK_CHECK_64(mask, cpu) { - lvm_info[i].pcpus[ii] = cpu; + for (cpu = 0; cpu < POWER_MGR_MAX_CPUS; cpu++) { + if (mask[cpu]) + lvm_info[i].pcpus[ii] = cpu;Same, should be mask[cpu] == 1.
Quoted 2 lines
Also, are two loops necessary?
I'll consolidate.
Quoted 21 lines
Quoted 21 lines
examples/vm_power_manager/channel_manager.c +1
-6} } }@@ -733,7 +742,7 @@ get_info_vm(const char *vm_name, struct vm_info *info) { struct virtual_machine_info *vm_info; unsigned i, channel_num = 0; - uint64_t mask; + char mask[POWER_MGR_MAX_CPUS]; vm_info = find_domain_by_name(vm_name); if (vm_info == NULL) {@@ -746,14 +755,19 @@ get_info_vm(const char *vm_name, struct vm_info *info) rte_spinlock_lock(&(vm_info->config_spinlock)); - mask = vm_info->channel_mask; - ITERATIVE_BITMASK_CHECK_64(mask, i) { - info->channels[channel_num].channel_num = i; - memcpy(info->channels[channel_num].channel_path, - vm_info->channels[i]->channel_path, UNIX_PATH_MAX); - info->channels[channel_num].status =
Quoted 9 lines
examples/vm_power_manager/channel_manager.c get_info_vm(const char *vm_name, struct vm_info *info) · line 760
Quoted 6 lines
examples/vm_power_manager/channel_manager.c +3
-2vm_info->channels[i]->status; - info->channels[channel_num].fd = vm_info->channels[i]->fd; - channel_num++; + memcpy(mask, (char *)vm_info->channel_mask, POWER_MGR_MAX_CPUS); + for (i = 0; i < POWER_MGR_MAX_CPUS; i++) { + if (mask[i] == 1) {Same - early exit would make things more readable.
Done
Quoted 32 lines
Quoted 32 lines
examples/vm_power_manager/channel_manager.c +12
-4+ info->channels[channel_num].channel_num = i; + memcpy(info->channels[channel_num].channel_path, + vm_info->channels[i]->channel_path, + UNIX_PATH_MAX); + info->channels[channel_num].status = + vm_info->channels[i]->status; + info->channels[channel_num].fd = + vm_info->channels[i]->fd; + channel_num++; + } } info->num_channels = channel_num;@@ -822,7 +836,7 @@ add_vm(const char *vm_name) } strncpy(new_domain->name, vm_name, sizeof(new_domain->name)); new_domain->name[sizeof(new_domain->name) - 1] = '\0'; - new_domain->channel_mask = 0; + memset(new_domain->channel_mask, 0, POWER_MGR_MAX_CPUS); new_domain->num_channels = 0; if (!virDomainIsActive(dom_ptr))@@ -940,18 +954,21 @@ void channel_manager_exit(void) { unsigned i; - uint64_t mask; + char mask[POWER_MGR_MAX_CPUS]; struct virtual_machine_info *vm_info; LIST_FOREACH(vm_info, &vm_list_head, vms_info) {rte_spinlock_lock (&(vm_info->config_spinlock)); - mask = vm_info->channel_mask; - ITERATIVE_BITMASK_CHECK_64(mask, i) { -remove_channel_from_monitor (vm_info->channels[i]);
Quoted 9 lines
examples/vm_power_manager/channel_manager.c void · line 966
Quoted 6 lines
examples/vm_power_manager/channel_manager.c +3
-2- close(vm_info->channels[i]->fd); - rte_free(vm_info->channels[i]); + memcpy(mask, (char *)vm_info->channel_mask,POWER_MGR_MAX_CPUS ); + for (i = 0; i < POWER_MGR_MAX_CPUS; i++) { + if (mask[i] == 1) {Same - early exit.
Done
Quoted 37 lines
Quoted 37 lines
examples/vm_power_manager/channel_manager.c +5
-0+remove_channel_from_monitor ( + vm_info->channels[i]); + close(vm_info->channels[i]->fd); + rte_free(vm_info->channels[i]); + } }rte_spinlock_unlock (&(vm_info->config_spinlock));diff --git
a/examples/vm_power_manager/channel_manager.hb/examples/vm_power_manager/channel_manager.hindexd948b304c..c2520ab6f100644examples/vm_power_manager/channel_manager.h +1
-1@@ -149,7 +149,7 @@ uint64_t get_pcpus_mask(struct channel_info*chan_info, unsigned vcpu); * - 0 on success. * - Negative on error. */ -int set_pcpus_mask (char *vm_name,unsigned vcpu,uint64_t core_mask); +int set_pcpus_mask (char *vm_name,unsigned int vcpu,char *core_mask); /*** Set the Physical CPU for the specified vCPU. examples/vm_power_manager/vm_power_cli.c +1
-1@@ -128,7 +128,7 @@ struct cmd_set_pcpu_mask_result { cmdline_fixed_string_t set_pcpu_mask; cmdline_fixed_string_t vm_name; uint8_t vcpu; - uint64_t core_mask; + char core_mask[POWER_MGR_MAX_CPUS]; };static void
examples/vm_power_manager/vm_power_cli.c +0
-0@@ -139,10 +139,10 @@ cmd_set_pcpu_mask_parsed(void *parsed_result,struct cmdline *cl, if (set_pcpus_mask(res->vm_name, res->vcpu, res->core_mask) == 0)cmdline_printf(cl, "Pinned
vCPU(%"PRId8")to pCPU core " - "mask(0x%"PRIx64")\n",res->vcpu,res->core_mask);
Quoted 10 lines
Quoted 8 lines
+ "\n",
res->vcpu);
else
cmdline_printf(cl, "Unable to pinvCPU(%"PRId8")to pCPU core " - "mask(0x%"PRIx64")\n",res->vcpu,res->core_mask); + "\n",res->vcpu);
}
cmdline_parse_token_string_tcmd_set_pcpu_mask=
Updated patch coming soon.
Thanks,
Dave.
v2 cover examples/power: allow use of more than 64 cores
In reply to David HuntFirst of all we convert all the relevant uint64_t's to char arrays. Then we remove the unneeded mask functions that were limited to 64 cores. Also extend the guest functionality and finally rause the number of supported cores to something more sensible, i.e. 256.
v2 changes:
* Updates out of Review by Anatoly.
* Indentation fixes
* resolved missing spinlock around config updates
* simplified some loops
[1/4] examples/power: change 64-bit masks to arrays [2/4] examples/power: remove mask functions [3/4] examples/power: allow vms to use lcores over 63 [4/4] examples/power: increase max cores to 256
PATCH v2 1/4 examples/power: change 64-bit masks to arrays
vm_power_manager currently makes use of uint64_t masks to keep track of cores in use, limiting use of the app to only being able to manage the first 64 cores in a multi-core system. Many modern systems have core counts greater than 64, so this limitation needs to be removed.
This patch converts the relevant 64-bit masks to character arrays.
--- examples/vm_power_manager/channel_manager.c | 89 ++++++++++++--------- examples/vm_power_manager/channel_manager.h | 2 +- examples/vm_power_manager/vm_power_cli.c | 6 +- 3 files changed, 55 insertions(+), 42 deletions(-)
examples/vm_power_manager/channel_manager.c +51 -38
@@ -29,14 +29,11 @@#include "channel_manager.h" #include "channel_commands.h" #include "channel_monitor.h" +#include "power_manager.h" #define RTE_LOGTYPE_CHANNEL_MANAGER RTE_LOGTYPE_USER1 -#define ITERATIVE_BITMASK_CHECK_64 (mask_u64b, i) \ - for (i = 0; mask_u64b; mask_u64b &= ~(1ULL << i++)) \ - if ((mask_u64b >> i) & 1) \ -/* Global pointer to libvirt connection */ static virConnectPtr global_vir_conn_ptr;
@@ -54,7 +51,7 @@ struct virtual_machine_info {char name[CHANNEL_MGR_MAX_NAME_LEN ];rte_atomic64_t pcpu_mask[CHANNEL_CMDS_MAX_CPUS ];struct channel_info *channels[CHANNEL_CMDS_MAX_VM_CHANNELS ]; -uint64_t channel_mask; +char channel_mask[POWER_MGR_MAX_CPUS ];uint8_t num_channels;enum vm_status status;virDomainPtr domainPtr;
@@ -135,12 +132,14 @@ update_pcpus_mask(struct virtual_machine_info *vm_info) }int -set_pcpus_mask (char *vm_name,unsigned vcpu,uint64_t core_mask) +set_pcpus_mask (char *vm_name,unsigned int vcpu,char *core_mask) {unsigned i =0 ;int flags =VIR_DOMAIN_AFFECT_LIVE |VIR_DOMAIN_AFFECT_CONFIG ;struct virtual_machine_info *vm_info; -uint64_t mask = core_mask; +char mask[POWER_MGR_MAX_CPUS ]; + +memcpy (mask, core_mask,POWER_MGR_MAX_CPUS );if (vcpu >=CHANNEL_CMDS_MAX_CPUS ) { RTE_LOG(ERR, CHANNEL_MANAGER, "vCPU(%u) exceeds max allowable(%d)\n",
@@ -156,8 +155,8 @@ set_pcpus_mask(char *vm_name, unsigned vcpu, uint64_t core_mask)if (!virDomainIsActive (vm_info->domainPtr)) {RTE_LOG (ERR ,CHANNEL_MANAGER ,"Unable to set vCPU(%u) to pCPU " -"mask(0x%" PRIx64") for VM '%s', VM is not active\n" , - vcpu, core_mask, vm_info->name); +" for VM '%s', VM is not active\n" , + vcpu, vm_info->name);return -1 ; }
@@ -167,22 +166,27 @@ set_pcpus_mask(char *vm_name, unsigned vcpu, uint64_t core_mask)return -1 ; }memset (global_cpumaps,0 ,CHANNEL_CMDS_MAX_CPUS * global_maplen); - ITERATIVE_BITMASK_CHECK_64(mask, i) { +for (i =0 ; i <POWER_MGR_MAX_CPUS ; i++) { +if (mask[i] !=1 ) +continue ;VIR_USE_CPU (global_cpumaps, i);if (i >= global_n_host_cpus) {RTE_LOG (ERR ,CHANNEL_MANAGER ,"CPU(%u) exceeds the available " -"number of CPUs(%u)\n" , i, global_n_host_cpus); +"number of CPUs(%u)\n" , + i, global_n_host_cpus);return -1 ; } }if (virDomainPinVcpuFlags (vm_info->domainPtr, vcpu, global_cpumaps, global_maplen, flags) <0 ) {RTE_LOG (ERR ,CHANNEL_MANAGER ,"Unable to set vCPU(%u) to pCPU " -"mask(0x%" PRIx64") for VM '%s'\n" , vcpu, core_mask, +" for VM '%s'\n" , vcpu, vm_info->name);return -1 ; } -rte_atomic64_set (&vm_info->pcpu_mask[vcpu], core_mask); +rte_spinlock_lock (&(vm_info->config_spinlock)); +memcpy (&vm_info->pcpu_mask[vcpu], mask,POWER_MGR_MAX_CPUS ); +rte_spinlock_unlock (&(vm_info->config_spinlock));return 0 ; }
@@ -190,7 +194,11 @@ set_pcpus_mask(char *vm_name, unsigned vcpu, uint64_t core_mask)int set_pcpu (char *vm_name,unsigned vcpu,unsigned core_num) { -uint64_t mask =1ULL << core_num; +char mask[POWER_MGR_MAX_CPUS ]; + +memset (mask,0 ,POWER_MGR_MAX_CPUS ); + + mask[core_num] =1 ;return set_pcpus_mask (vm_name, vcpu, mask); }
@@ -211,7 +219,7 @@ static inline int channel_exists(struct virtual_machine_info *vm_info, unsigned channel_num) {rte_spinlock_lock (&(vm_info->config_spinlock)); -if (vm_info->channel_mask & (1ULL << channel_num)) { +if (vm_info->channel_mask[channel_num] ==1 ) {rte_spinlock_unlock (&(vm_info->config_spinlock));return 1 ; }
@@ -343,7 +351,7 @@ setup_channel_info(struct virtual_machine_info **vm_info_dptr, }rte_spinlock_lock (&(vm_info->config_spinlock)); vm_info->num_channels++; - vm_info->channel_mask |=1ULL << channel_num; + vm_info->channel_mask[channel_num] =1 ; vm_info->channels[channel_num] = chan_info; chan_info->status =CHANNEL_MGR_CHANNEL_CONNECTED ;rte_spinlock_unlock (&(vm_info->config_spinlock));
@@ -590,7 +598,7 @@ remove_channel(struct channel_info **chan_info_dptr) vm_info = (struct virtual_machine_info *)chan_info->priv_info;rte_spinlock_lock (&(vm_info->config_spinlock)); - vm_info->channel_mask &= ~(1ULL << chan_info->channel_num); + vm_info->channel_mask[chan_info->channel_num] =0 ; vm_info->num_channels--;rte_spinlock_unlock (&(vm_info->config_spinlock));
@@ -603,7 +611,7 @@ set_channel_status_all(const char *vm_name, enum channel_status status) {struct virtual_machine_info *vm_info;unsigned i; -uint64_t mask; +char mask[POWER_MGR_MAX_CPUS ];int num_channels_changed =0 ; if (!(status == CHANNEL_MGR_CHANNEL_CONNECTED ||
@@ -619,8 +627,10 @@ set_channel_status_all(const char *vm_name, enum channel_status status) }rte_spinlock_lock (&(vm_info->config_spinlock)); - mask = vm_info->channel_mask; - ITERATIVE_BITMASK_CHECK_64(mask, i) { +memcpy (mask, (char *)vm_info->channel_mask,POWER_MGR_MAX_CPUS ); +for (i =0 ; i <POWER_MGR_MAX_CPUS ; i++) { +if (mask[i] !=1 ) +continue ; vm_info->channels[i]->status = status; num_channels_changed++; }
@@ -665,8 +675,7 @@ get_all_vm(int *num_vm, int *num_vcpu)virNodeInfo node_info;virDomainPtr *domptr; -uint64_t mask; -int i, ii, numVcpus[MAX_VCPUS ], cpu, n_vcpus; +int i, ii, numVcpus[MAX_VCPUS ], n_vcpus;unsigned int jj;const char *vm_name; unsigned int domain_flags = VIR_CONNECT_LIST_DOMAINS_RUNNING |
@@ -714,16 +723,12 @@ get_all_vm(int *num_vm, int *num_vcpu)/* Save pcpu in use by libvirt VMs */ for (ii =0 ; ii < n_vcpus; ii++) { - mask =0 ;for (jj =0 ; jj < global_n_host_cpus; jj++) {if (VIR_CPU_USABLE (global_cpumaps, global_maplen, ii, jj) >0 ) { - mask |=1ULL << jj; + lvm_info[i].pcpus[ii] = jj; } } - ITERATIVE_BITMASK_CHECK_64(mask, cpu) { - lvm_info[i].pcpus[ii] = cpu; - } } } }
@@ -733,7 +738,7 @@ get_info_vm(const char *vm_name, struct vm_info *info) {struct virtual_machine_info *vm_info;unsigned i, channel_num =0 ; -uint64_t mask; +char mask[POWER_MGR_MAX_CPUS ]; vm_info =find_domain_by_name (vm_name);if (vm_info ==NULL ) {
@@ -746,13 +751,18 @@ get_info_vm(const char *vm_name, struct vm_info *info)rte_spinlock_lock (&(vm_info->config_spinlock)); - mask = vm_info->channel_mask; - ITERATIVE_BITMASK_CHECK_64(mask, i) { +memcpy (mask, (char *)vm_info->channel_mask,POWER_MGR_MAX_CPUS ); +for (i =0 ; i <POWER_MGR_MAX_CPUS ; i++) { +if (mask[i] !=1 ) +continue ; info->channels[channel_num].channel_num = i;memcpy (info->channels[channel_num].channel_path, - vm_info->channels[i]->channel_path,UNIX_PATH_MAX ); - info->channels[channel_num].status = vm_info->channels[i]->status; - info->channels[channel_num].fd = vm_info->channels[i]->fd; + vm_info->channels[i]->channel_path, +UNIX_PATH_MAX ); + info->channels[channel_num].status = + vm_info->channels[i]->status; + info->channels[channel_num].fd = + vm_info->channels[i]->fd; channel_num++; }
@@ -822,7 +832,7 @@ add_vm(const char *vm_name) }strncpy (new_domain->name, vm_name,sizeof (new_domain->name)); new_domain->name[sizeof (new_domain->name) -1 ] ='\0' ; - new_domain->channel_mask =0 ; +memset (new_domain->channel_mask,0 ,POWER_MGR_MAX_CPUS ); new_domain->num_channels =0 ;if (!virDomainIsActive (dom_ptr))
@@ -940,16 +950,19 @@ voidchannel_manager_exit (void) {unsigned i; -uint64_t mask; +char mask[POWER_MGR_MAX_CPUS ];struct virtual_machine_info *vm_info; LIST_FOREACH(vm_info, &vm_list_head, vms_info) {rte_spinlock_lock (&(vm_info->config_spinlock)); - mask = vm_info->channel_mask; - ITERATIVE_BITMASK_CHECK_64(mask, i) { -remove_channel_from_monitor (vm_info->channels[i]); +memcpy (mask, (char *)vm_info->channel_mask,POWER_MGR_MAX_CPUS ); +for (i =0 ; i <POWER_MGR_MAX_CPUS ; i++) { +if (mask[i] !=1 ) +continue ; +remove_channel_from_monitor ( + vm_info->channels[i]);close (vm_info->channels[i]->fd);rte_free (vm_info->channels[i]); }
examples/vm_power_manager/channel_manager.h +1 -1
@@ -149,7 +149,7 @@ uint64_t get_pcpus_mask(struct channel_info *chan_info, unsigned vcpu);* - 0 on success. * - Negative on error. */ -int set_pcpus_mask (char *vm_name,unsigned vcpu,uint64_t core_mask); +int set_pcpus_mask (char *vm_name,unsigned int vcpu,char *core_mask);/** * Set the Physical CPU for the specified vCPU.
examples/vm_power_manager/vm_power_cli.c +3 -3
@@ -128,7 +128,7 @@ struct cmd_set_pcpu_mask_result {cmdline_fixed_string_t set_pcpu_mask;cmdline_fixed_string_t vm_name;uint8_t vcpu; -uint64_t core_mask; +char core_mask[POWER_MGR_MAX_CPUS ]; }; static void
@@ -139,10 +139,10 @@ cmd_set_pcpu_mask_parsed(void *parsed_result, struct cmdline *cl,if (set_pcpus_mask (res->vm_name, res->vcpu, res->core_mask) ==0 )cmdline_printf (cl,"Pinned vCPU(%" PRId8") to pCPU core " -"mask(0x%" PRIx64")\n" , res->vcpu, res->core_mask); +"\n" , res->vcpu);else cmdline_printf (cl,"Unable to pin vCPU(%" PRId8") to pCPU core " -"mask(0x%" PRIx64")\n" , res->vcpu, res->core_mask); +"\n" , res->vcpu); } cmdline_parse_token_string_t cmd_set_pcpu_mask =
Signature · 1 line
2.17.1
Re: [PATCH v2 1/4] examples/power: change 64-bit masks to arrays
Quoted 9 lines · David Hunt
vm_power_managercurrently makes use ofuint64_tmasks to keep track of cores in use, limiting use of the app to only being able to manage the first 64 cores in a multi-core system. Many modern systems have core counts greater than 64, so this limitation needs to be removed.This patch converts the relevant 64-bit masks to character arrays.
Signed-off-by David Hunt <[email protected]>---
Signature · 2 lines
Thanks, Anatoly
PATCH v2 2/4 examples/power: remove mask functions
In reply to David Huntsince we're moving to allowing greater than 64 cores, the mask functions that use uint64_t to perform functions on a masked set of cores are no longer needed, so removing them.
--- examples/vm_power_manager/channel_monitor.c | 24 ---- examples/vm_power_manager/power_manager.c | 68 --------- examples/vm_power_manager/vm_power_cli.c | 149 -------------------- 3 files changed, 241 deletions(-)
examples/vm_power_manager/channel_monitor.c +0 -24
@@ -681,30 +681,6 @@ process_request(struct channel_packet *pkt, struct channel_info *chan_info)default :break ; } - }else { -switch (pkt->unit) { -case (CPU_POWER_SCALE_MIN ): -power_manager_scale_mask_min (core_mask); -break ; -case (CPU_POWER_SCALE_MAX ): -power_manager_scale_mask_max (core_mask); -break ; -case (CPU_POWER_SCALE_DOWN ): -power_manager_scale_mask_down (core_mask); -break ; -case (CPU_POWER_SCALE_UP ): -power_manager_scale_mask_up (core_mask); -break ; -case (CPU_POWER_ENABLE_TURBO ): -power_manager_enable_turbo_mask (core_mask); -break ; -case (CPU_POWER_DISABLE_TURBO ): -power_manager_disable_turbo_mask (core_mask); -break ; -default : -break ; - } - } }
examples/vm_power_manager/power_manager.c +0 -68
80 lines not on this page · raw message
examples/vm_power_manager/vm_power_cli.c +0 -149
170 lines not on this page · raw message
Signature · 1 line
2.17.1
Re: [PATCH v2 2/4] examples/power: remove mask functions
Quoted 6 lines · David Hunt
since we're moving to allowing greater than 64 cores, the mask functions that use
uint64_tto perform functions on a masked set of cores are no longer needed, so removing them.Signed-off-by David Hunt <[email protected]>---
Signature · 2 lines
Thanks, Anatoly
PATCH v2 3/4 examples/power: allow vms to use lcores over 63
In reply to David HuntExtending the functionality to allow vms to power manage cores beyond 63.
--- examples/vm_power_manager/channel_manager.c | 74 +++++++++------------ examples/vm_power_manager/channel_manager.h | 30 ++------- examples/vm_power_manager/channel_monitor.c | 56 ++++++---------- examples/vm_power_manager/vm_power_cli.c | 4 +- 4 files changed, 57 insertions(+), 107 deletions(-)
examples/vm_power_manager/channel_manager.c +31 -43
@@ -49,7 +49,7 @@ static bool global_hypervisor_available;*/ struct virtual_machine_info {char name[CHANNEL_MGR_MAX_NAME_LEN ]; -rte_atomic64_t pcpu_mask[CHANNEL_CMDS_MAX_CPUS ]; +uint16_t pcpu_map[CHANNEL_CMDS_MAX_CPUS ];struct channel_info *channels[CHANNEL_CMDS_MAX_VM_CHANNELS ];char channel_mask[POWER_MGR_MAX_CPUS ];uint8_t num_channels;
@@ -79,7 +79,6 @@ update_pcpus_mask(struct virtual_machine_info *vm_info)virVcpuInfoPtr cpuinfo;unsigned i, j;int n_vcpus; -uint64_t mask;memset (global_cpumaps,0 ,CHANNEL_CMDS_MAX_CPUS *global_maplen);
@@ -120,26 +119,23 @@ update_pcpus_mask(struct virtual_machine_info *vm_info) vm_info->info.nrVirtCpu = n_vcpus; }for (i =0 ; i < vm_info->info.nrVirtCpu; i++) { - mask =0 ;for (j =0 ; j < global_n_host_cpus; j++) { -if (VIR_CPU_USABLE (global_cpumaps, global_maplen, i, j) >0 ) { - mask |=1ULL << j; - } +if (VIR_CPU_USABLE (global_cpumaps, + global_maplen, i, j) <=0 ) +continue ; +rte_spinlock_lock (&(vm_info->config_spinlock)); + vm_info->pcpu_map[i] = j; +rte_spinlock_unlock (&(vm_info->config_spinlock)); } -rte_atomic64_set (&vm_info->pcpu_mask[i], mask); }return 0 ; }int -set_pcpus_mask (char *vm_name,unsigned int vcpu,char *core_mask) +set_pcpu (char *vm_name,unsigned int vcpu,unsigned int pcpu) { -unsigned i =0 ;int flags =VIR_DOMAIN_AFFECT_LIVE |VIR_DOMAIN_AFFECT_CONFIG ;struct virtual_machine_info *vm_info; -char mask[POWER_MGR_MAX_CPUS ]; - -memcpy (mask, core_mask,POWER_MGR_MAX_CPUS );if (vcpu >=CHANNEL_CMDS_MAX_CPUS ) { RTE_LOG(ERR, CHANNEL_MANAGER, "vCPU(%u) exceeds max allowable(%d)\n",
@@ -166,17 +162,16 @@ set_pcpus_mask(char *vm_name, unsigned int vcpu, char *core_mask)return -1 ; }memset (global_cpumaps,0 ,CHANNEL_CMDS_MAX_CPUS * global_maplen); -for (i =0 ; i <POWER_MGR_MAX_CPUS ; i++) { -if (mask[i] !=1 ) -continue ; -VIR_USE_CPU (global_cpumaps, i); -if (i >= global_n_host_cpus) { -RTE_LOG (ERR ,CHANNEL_MANAGER ,"CPU(%u) exceeds the available " -"number of CPUs(%u)\n" , - i, global_n_host_cpus); -return -1 ; - } + +VIR_USE_CPU (global_cpumaps, pcpu); + +if (pcpu >= global_n_host_cpus) { +RTE_LOG (ERR ,CHANNEL_MANAGER ,"CPU(%u) exceeds the available " +"number of CPUs(%u)\n" , + pcpu, global_n_host_cpus); +return -1 ; } +if (virDomainPinVcpuFlags (vm_info->domainPtr, vcpu, global_cpumaps, global_maplen, flags) <0 ) { RTE_LOG(ERR, CHANNEL_MANAGER, "Unable to set vCPU(%u) to pCPU "
@@ -185,33 +180,24 @@ set_pcpus_mask(char *vm_name, unsigned int vcpu, char *core_mask)return -1 ; }rte_spinlock_lock (&(vm_info->config_spinlock)); -memcpy (&vm_info->pcpu_mask[vcpu], mask,POWER_MGR_MAX_CPUS ); + vm_info->pcpu_map[vcpu] = pcpu;rte_spinlock_unlock (&(vm_info->config_spinlock));return 0 ; - -} - -int -set_pcpu (char *vm_name,unsigned vcpu,unsigned core_num) -{ -char mask[POWER_MGR_MAX_CPUS ]; - -memset (mask,0 ,POWER_MGR_MAX_CPUS ); - - mask[core_num] =1 ; - -return set_pcpus_mask (vm_name, vcpu, mask); } -uint64_t -get_pcpus_mask (struct channel_info *chan_info,unsigned vcpu) +uint16_t +get_pcpu (struct channel_info *chan_info,unsigned int vcpu) {struct virtual_machine_info *vm_info = (struct virtual_machine_info *)chan_info->priv_info; -if (global_hypervisor_available && (vm_info !=NULL )) -return rte_atomic64_read (&vm_info->pcpu_mask[vcpu]); -else +if (global_hypervisor_available && (vm_info !=NULL )) { +uint16_t pcpu; +rte_spinlock_lock (&(vm_info->config_spinlock)); + pcpu = vm_info->pcpu_map[vcpu]; +rte_spinlock_unlock (&(vm_info->config_spinlock)); +return pcpu; + }else return 0 ; }
@@ -771,9 +757,11 @@ get_info_vm(const char *vm_name, struct vm_info *info)rte_spinlock_unlock (&(vm_info->config_spinlock));memcpy (info->name, vm_info->name,sizeof (vm_info->name)); +rte_spinlock_lock (&(vm_info->config_spinlock));for (i =0 ; i < info->num_vcpus; i++) { - info->pcpu_mask[i] =rte_atomic64_read (&vm_info->pcpu_mask[i]); + info->pcpu_map[i] = vm_info->pcpu_map[i]; } +rte_spinlock_unlock (&(vm_info->config_spinlock));return 0 ; }
@@ -823,7 +811,7 @@ add_vm(const char *vm_name) }for (i =0 ; i <CHANNEL_CMDS_MAX_CPUS ; i++) { -rte_atomic64_init (&new_domain->pcpu_mask[i]); + new_domain->pcpu_map[i] =0 ; }if (update_pcpus_mask (new_domain) <0 ) {RTE_LOG (ERR ,CHANNEL_MANAGER ,"Error getting physical CPU pinning\n" );
examples/vm_power_manager/channel_manager.h +5 -25
@@ -40,7 +40,6 @@ struct sockaddr_un _sockaddr_un;#define MAX_CLIENTS 64#define MAX_VCPUS 20 -struct libvirt_vm_info {const char *vm_name;unsigned int pcpus[MAX_VCPUS ];
@@ -82,7 +81,7 @@ struct channel_info {struct vm_info {char name[CHANNEL_MGR_MAX_NAME_LEN ];/**< VM name */ enum vm_status status;/**< libvirt status */ -uint64_t pcpu_mask[CHANNEL_CMDS_MAX_CPUS ];/**< pCPU mask for each vCPU */ +uint16_t pcpu_map[CHANNEL_CMDS_MAX_CPUS ];/**< pCPU map to vCPU */ unsigned num_vcpus;/**< number of vCPUS */ struct channel_info channels[CHANNEL_MGR_MAX_CHANNELS ];/**< Array of channel_info */ unsigned num_channels;/**< Number of channels */
@@ -115,8 +114,7 @@ int channel_manager_init(const char *path);void channel_manager_exit (void );/** -* Get the Physical CPU mask for VM lcore channel(vcpu), result is assigned to -* core_mask. +* Get the Physical CPU for VM lcore channel(vcpu). * It is not thread-safe. * * @param chan_info
@@ -130,26 +128,7 @@ void channel_manager_exit(void);* - 0 on error. * - >0 on success. */ -uint64_t get_pcpus_mask (struct channel_info *chan_info,unsigned vcpu); - -/** -* Set the Physical CPU mask for the specified vCPU. -* It is not thread-safe. -* -* @param name -* Virtual Machine name to lookup -* -* @param vcpu -* The virtual CPU to set. -* -* @param core_mask -* The core mask of the physical CPU(s) to bind the vCPU -* -* @return -* - 0 on success. -* - Negative on error. -*/ -int set_pcpus_mask (char *vm_name,unsigned int vcpu,char *core_mask); +uint16_t get_pcpu (struct channel_info *chan_info,unsigned int vcpu);/** * Set the Physical CPU for the specified vCPU.
@@ -168,7 +147,8 @@ int set_pcpus_mask(char *vm_name, unsigned int vcpu, char *core_mask);* - 0 on success. * - Negative on error. */ -int set_pcpu (char *vm_name,unsigned vcpu,unsigned core_num); +int set_pcpu (char *vm_name,unsigned int vcpu,unsigned int pcpu); +/** * Add a VM as specified by name to the Channel Manager. The name must * correspond to a valid libvirt domain name.
examples/vm_power_manager/channel_monitor.c +19 -37
88 lines not on this page · raw message
examples/vm_power_manager/vm_power_cli.c +2 -2
@@ -95,8 +95,8 @@ cmd_show_vm_parsed(void *parsed_result, struct cmdline *cl, }cmdline_printf (cl,"Virtual CPU(s): %u\n" , info.num_vcpus);for (i =0 ; i < info.num_vcpus; i++) { -cmdline_printf (cl," [%u]: Physical CPU Mask 0x%" PRIx64"\n" , i, - info.pcpu_mask[i]); +cmdline_printf (cl," [%u]: Physical CPU %d\n" , i, + info.pcpu_map[i]); } }
Signature · 1 line
2.17.1
Re: [PATCH v2 3/4] examples/power: allow vms to use lcores over 63
Quoted 36 lines · David Hunt
Extending the functionality to allow vms to power manage cores beyond 63.
Signed-off-by David Hunt <[email protected]>--- examples/vm_power_manager/channel_manager.c | 74 +++++++++------------ examples/vm_power_manager/channel_manager.h | 30 ++------- examples/vm_power_manager/channel_monitor.c | 56 ++++++---------- examples/vm_power_manager/vm_power_cli.c | 4 +- 4 files changed, 57 insertions(+), 107 deletions(-)examples/vm_power_manager/channel_manager.c +1
-3@@ -49,7 +49,7 @@ static bool global_hypervisor_available;*/ struct virtual_machine_info {char name[CHANNEL_MGR_MAX_NAME_LEN ]; -rte_atomic64_t pcpu_mask[CHANNEL_CMDS_MAX_CPUS ]; +uint16_t pcpu_map[CHANNEL_CMDS_MAX_CPUS ];struct channel_info *channels[CHANNEL_CMDS_MAX_VM_CHANNELS ];char channel_mask[POWER_MGR_MAX_CPUS ];uint8_t num_channels;@@ -79,7 +79,6 @@ update_pcpus_mask(struct virtual_machine_info *vm_info)virVcpuInfoPtr cpuinfo;unsigned i, j;int n_vcpus; -uint64_t mask;memset (global_cpumaps,0 ,CHANNEL_CMDS_MAX_CPUS *global_maplen);@@ -120,26 +119,23 @@ update_pcpus_mask(struct virtual_machine_info *vm_info) vm_info->info.nrVirtCpu = n_vcpus; }for (i =0 ; i < vm_info->info.nrVirtCpu; i++) { - mask =0 ;
examples/vm_power_manager/channel_manager.c update_pcpus_mask(struct virtual_machine_info *vm_info) · line 128
Quoted 10 lines
examples/vm_power_manager/channel_manager.c +6
-3for (j =0 ; j < global_n_host_cpus; j++) { -if (VIR_CPU_USABLE (global_cpumaps, global_maplen, i, j) >0 ) { - mask |=1ULL << j; - } +if (VIR_CPU_USABLE (global_cpumaps, + global_maplen, i, j) <=0 ) +continue ; +rte_spinlock_lock (&(vm_info->config_spinlock)); + vm_info->pcpu_map[i] = j; +rte_spinlock_unlock (&(vm_info->config_spinlock));
This is not an equivalent replacement, because something can happen inbetween the unlock and subsequent lock. There's no need to lock-unlock it on every iteration anyway - just lock it before the for (i = 0...) and unlock it right before return.
Quoted 10 lines
examples/vm_power_manager/channel_manager.c +1
-2} -rte_atomic64_set (&vm_info->pcpu_mask[i], mask); }return 0 ; }int -set_pcpus_mask (char *vm_name,unsigned int vcpu,char *core_mask) +set_pcpu (char *vm_name,unsigned int vcpu,unsigned int pcpu) {
<snip>
examples/vm_power_manager/channel_manager.h struct sockaddr_un _sockaddr_un; · line 43
Quoted 8 lines
examples/vm_power_manager/channel_manager.h +0
-1@@ -40,7 +40,6 @@ struct sockaddr_un _sockaddr_un;#define MAX_CLIENTS 64#define MAX_VCPUS 20 -struct libvirt_vm_info {
Unintended whitespace change?
Quoted 6 lines
examples/vm_power_manager/channel_manager.h +0
-0const char *vm_name;unsigned int pcpus[MAX_VCPUS ];@@ -82,7 +81,7 @@ struct channel_info {struct vm_info {char name[CHANNEL_MGR_MAX_NAME_LEN ];/**< VM name */ enum vm_status status;/**< libvirt status */
Signature · 2 lines
Thanks, Anatoly
Re: [PATCH v2 3/4] examples/power: allow vms to use lcores over 63
Hi Anatoly,
Quoted 48 lines · Anatoly
Quoted 38 lines · David Hunt
Extending the functionality to allow vms to power manage cores beyond 63.
Signed-off-by David Hunt <[email protected]>---
examples/vm_power_manager/channel_manager.c | 74 +++++++++------------ examples/vm_power_manager/channel_manager.h | 30 ++------- examples/vm_power_manager/channel_monitor.c | 56 ++++++---------- examples/vm_power_manager/vm_power_cli.c | 4 +- 4 files changed, 57 insertions(+), 107 deletions(-)examples/vm_power_manager/channel_manager.c +1
-2@@ -49,7 +49,7 @@ static bool global_hypervisor_available;*/ struct virtual_machine_info { char name[CHANNEL_MGR_MAX_NAME_LEN]; - rte_atomic64_t pcpu_mask[CHANNEL_CMDS_MAX_CPUS]; + uint16_t pcpu_map[CHANNEL_CMDS_MAX_CPUS]; struct channel_info *channels[CHANNEL_CMDS_MAX_VM_CHANNELS]; char channel_mask[POWER_MGR_MAX_CPUS]; uint8_t num_channels;@@ -79,7 +79,6 @@ update_pcpus_mask(struct virtual_machine_info *vm_info) virVcpuInfoPtr cpuinfo; unsigned i, j; int n_vcpus; - uint64_t mask; memset(global_cpumaps, 0, CHANNEL_CMDS_MAX_CPUS*global_maplen); @@ -120,26 +119,23 @@ update_pcpus_mask(struct virtual_machine_info *vm_info)
vm_info->info.nrVirtCpu=n_vcpus;
}
for (i = 0; i <vm_info->info.nrVirtCpu; i++) {
- mask = 0;
for (j = 0; j <global_n_host_cpus; j++) {
- if (VIR_CPU_USABLE(global_cpumaps, global_maplen, i, j)Quoted 1 line
0) {
Quoted 8 lines
- mask |= 1ULL << j;
- }
+ if (VIR_CPU_USABLE(global_cpumaps, + global_maplen, i, j)<= 0) + continue;
+ rte_spinlock_lock(&(vm_info->config_spinlock)); + vm_info->pcpu_map[i] = j;
+ rte_spinlock_unlock(&(vm_info->config_spinlock));
Quoted 2 lines
This is not an equivalent replacement, because something can happen inbetween the unlock and subsequent lock. There's no need to lock-unlock it on every iteration anyway - just lock it before the for (i = 0...) and unlock it right before return.
Will fix in next revision.
Quoted 12 lines
Quoted 9 lines
}
-rte_atomic64_set(&vm_info->pcpu_mask[i], mask); }
return 0;
}
intexamples/vm_power_manager/channel_manager.c +1
-1-set_pcpus_mask(char *vm_name, unsigned int vcpu, char *core_mask) +set_pcpu(char *vm_name, unsigned int vcpu, unsigned int pcpu){
<snip>
Quoted 10 lines
Quoted 7 lines
examples/vm_power_manager/channel_manager.h +0
-0@@ -40,7 +40,6 @@ struct sockaddr_un _sockaddr_un; #define MAX_CLIENTS 64 #define MAX_VCPUS 20-
structlibvirt_vm_info{Unintended whitespace change?
Yes, will address.
Quoted 7 lines
Quoted 6 lines
const char
*vm_name;
unsigned int pcpus[MAX_VCPUS];examples/vm_power_manager/channel_manager.h +0
-0@@ -82,7 +81,7 @@ struct channel_info { struct vm_info {char name[CHANNEL_MGR_MAX_NAME_LEN]; /**< VM name */ enum
vm_statusstatus; /**< libvirt status */
Thanks,
Dave.
PATCH v2 4/4 examples/power: increase max cores to 256
In reply to David HuntIncrease the number of addressable cores from 64 to 256. Also remove the warning that incresing this number beyond 64 will cause problems (because of the previous use of uint64_t masks). Now this number can be increased significantly without causing problems.
--- examples/vm_power_manager/channel_manager.h | 8 ++------ examples/vm_power_manager/power_manager.h | 2 +- 2 files changed, 3 insertions(+), 7 deletions(-)
examples/vm_power_manager/channel_manager.h +2 -6
@@ -14,17 +14,13 @@ extern "C" {#include <rte_atomic.h> /* Maximum number of CPUs */ -#define CHANNEL_CMDS_MAX_CPUS 64 -#if CHANNEL_CMDS_MAX_CPUS >64 -#error Maximum number of cores is 64, overflow is guaranteed to \ - cause problems with VM Power Management -#endif +#define CHANNEL_CMDS_MAX_CPUS 256/* Maximum name length including '\0' terminator */ #define CHANNEL_MGR_MAX_NAME_LEN 64/* Maximum number of channels to each Virtual Machine */ -#define CHANNEL_MGR_MAX_CHANNELS 64 +#define CHANNEL_MGR_MAX_CHANNELS 256/* Hypervisor Path for libvirt(qemu/KVM) */ #define CHANNEL_MGR_DEFAULT_HV_PATH "qemu:///system"
examples/vm_power_manager/power_manager.h +1 -1
@@ -33,7 +33,7 @@ core_info_init(void);#define RTE_LOGTYPE_POWER_MANAGER RTE_LOGTYPE_USER1/* Maximum number of CPUS to manage */ -#define POWER_MGR_MAX_CPUS 64 +#define POWER_MGR_MAX_CPUS 256/** * Initialize power management. * Initializes resources and verifies the number of CPUs on the system.
Signature · 1 line
2.17.1
v3 cover examples/power: allow use of more than 64 cores
In reply to David HuntFirst of all we convert all the relevant uint64_t's to char arrays. Then we remove the unneeded mask functions that were limited to 64 cores. Also extend the guest functionality and finally rause the number of supported cores to something more sensible, i.e. 256.
v2 changes:
* Updates out of Review by Anatoly.
* Indentation fixes
* resolved missing spinlock around config updates
* simplified some loops
* fixed bug in non-equivalent code
v3 changes:
* Moved a spinlock to a more appropriate place. Was in a for-loop, made more sense to have it outside.
[1/4] examples/power: change 64-bit masks to arrays [2/4] examples/power: remove mask functions [3/4] examples/power: allow vms to use lcores over 63 [4/4] examples/power: increase max cores to 256
PATCH v3 1/4 examples/power: change 64-bit masks to arrays
vm_power_manager currently makes use of uint64_t masks to keep track of cores in use, limiting use of the app to only being able to manage the first 64 cores in a multi-core system. Many modern systems have core counts greater than 64, so this limitation needs to be removed.
This patch converts the relevant 64-bit masks to character arrays.
--- examples/vm_power_manager/channel_manager.c | 89 ++++++++++++--------- examples/vm_power_manager/channel_manager.h | 2 +- examples/vm_power_manager/vm_power_cli.c | 6 +- 3 files changed, 55 insertions(+), 42 deletions(-)
examples/vm_power_manager/channel_manager.c +51 -38
@@ -29,14 +29,11 @@#include "channel_manager.h" #include "channel_commands.h" #include "channel_monitor.h" +#include "power_manager.h" #define RTE_LOGTYPE_CHANNEL_MANAGER RTE_LOGTYPE_USER1 -#define ITERATIVE_BITMASK_CHECK_64 (mask_u64b, i) \ - for (i = 0; mask_u64b; mask_u64b &= ~(1ULL << i++)) \ - if ((mask_u64b >> i) & 1) \ -/* Global pointer to libvirt connection */ static virConnectPtr global_vir_conn_ptr;
@@ -54,7 +51,7 @@ struct virtual_machine_info {char name[CHANNEL_MGR_MAX_NAME_LEN ];rte_atomic64_t pcpu_mask[CHANNEL_CMDS_MAX_CPUS ];struct channel_info *channels[CHANNEL_CMDS_MAX_VM_CHANNELS ]; -uint64_t channel_mask; +char channel_mask[POWER_MGR_MAX_CPUS ];uint8_t num_channels;enum vm_status status;virDomainPtr domainPtr;
@@ -135,12 +132,14 @@ update_pcpus_mask(struct virtual_machine_info *vm_info) }int -set_pcpus_mask (char *vm_name,unsigned vcpu,uint64_t core_mask) +set_pcpus_mask (char *vm_name,unsigned int vcpu,char *core_mask) {unsigned i =0 ;int flags =VIR_DOMAIN_AFFECT_LIVE |VIR_DOMAIN_AFFECT_CONFIG ;struct virtual_machine_info *vm_info; -uint64_t mask = core_mask; +char mask[POWER_MGR_MAX_CPUS ]; + +memcpy (mask, core_mask,POWER_MGR_MAX_CPUS );if (vcpu >=CHANNEL_CMDS_MAX_CPUS ) { RTE_LOG(ERR, CHANNEL_MANAGER, "vCPU(%u) exceeds max allowable(%d)\n",
@@ -156,8 +155,8 @@ set_pcpus_mask(char *vm_name, unsigned vcpu, uint64_t core_mask)if (!virDomainIsActive (vm_info->domainPtr)) {RTE_LOG (ERR ,CHANNEL_MANAGER ,"Unable to set vCPU(%u) to pCPU " -"mask(0x%" PRIx64") for VM '%s', VM is not active\n" , - vcpu, core_mask, vm_info->name); +" for VM '%s', VM is not active\n" , + vcpu, vm_info->name);return -1 ; }
@@ -167,22 +166,27 @@ set_pcpus_mask(char *vm_name, unsigned vcpu, uint64_t core_mask)return -1 ; }memset (global_cpumaps,0 ,CHANNEL_CMDS_MAX_CPUS * global_maplen); - ITERATIVE_BITMASK_CHECK_64(mask, i) { +for (i =0 ; i <POWER_MGR_MAX_CPUS ; i++) { +if (mask[i] !=1 ) +continue ;VIR_USE_CPU (global_cpumaps, i);if (i >= global_n_host_cpus) {RTE_LOG (ERR ,CHANNEL_MANAGER ,"CPU(%u) exceeds the available " -"number of CPUs(%u)\n" , i, global_n_host_cpus); +"number of CPUs(%u)\n" , + i, global_n_host_cpus);return -1 ; } }if (virDomainPinVcpuFlags (vm_info->domainPtr, vcpu, global_cpumaps, global_maplen, flags) <0 ) {RTE_LOG (ERR ,CHANNEL_MANAGER ,"Unable to set vCPU(%u) to pCPU " -"mask(0x%" PRIx64") for VM '%s'\n" , vcpu, core_mask, +" for VM '%s'\n" , vcpu, vm_info->name);return -1 ; } -rte_atomic64_set (&vm_info->pcpu_mask[vcpu], core_mask); +rte_spinlock_lock (&(vm_info->config_spinlock)); +memcpy (&vm_info->pcpu_mask[vcpu], mask,POWER_MGR_MAX_CPUS ); +rte_spinlock_unlock (&(vm_info->config_spinlock));return 0 ; }
@@ -190,7 +194,11 @@ set_pcpus_mask(char *vm_name, unsigned vcpu, uint64_t core_mask)int set_pcpu (char *vm_name,unsigned vcpu,unsigned core_num) { -uint64_t mask =1ULL << core_num; +char mask[POWER_MGR_MAX_CPUS ]; + +memset (mask,0 ,POWER_MGR_MAX_CPUS ); + + mask[core_num] =1 ;return set_pcpus_mask (vm_name, vcpu, mask); }
@@ -211,7 +219,7 @@ static inline int channel_exists(struct virtual_machine_info *vm_info, unsigned channel_num) {rte_spinlock_lock (&(vm_info->config_spinlock)); -if (vm_info->channel_mask & (1ULL << channel_num)) { +if (vm_info->channel_mask[channel_num] ==1 ) {rte_spinlock_unlock (&(vm_info->config_spinlock));return 1 ; }
@@ -343,7 +351,7 @@ setup_channel_info(struct virtual_machine_info **vm_info_dptr, }rte_spinlock_lock (&(vm_info->config_spinlock)); vm_info->num_channels++; - vm_info->channel_mask |=1ULL << channel_num; + vm_info->channel_mask[channel_num] =1 ; vm_info->channels[channel_num] = chan_info; chan_info->status =CHANNEL_MGR_CHANNEL_CONNECTED ;rte_spinlock_unlock (&(vm_info->config_spinlock));
@@ -590,7 +598,7 @@ remove_channel(struct channel_info **chan_info_dptr) vm_info = (struct virtual_machine_info *)chan_info->priv_info;rte_spinlock_lock (&(vm_info->config_spinlock)); - vm_info->channel_mask &= ~(1ULL << chan_info->channel_num); + vm_info->channel_mask[chan_info->channel_num] =0 ; vm_info->num_channels--;rte_spinlock_unlock (&(vm_info->config_spinlock));
@@ -603,7 +611,7 @@ set_channel_status_all(const char *vm_name, enum channel_status status) {struct virtual_machine_info *vm_info;unsigned i; -uint64_t mask; +char mask[POWER_MGR_MAX_CPUS ];int num_channels_changed =0 ; if (!(status == CHANNEL_MGR_CHANNEL_CONNECTED ||
@@ -619,8 +627,10 @@ set_channel_status_all(const char *vm_name, enum channel_status status) }rte_spinlock_lock (&(vm_info->config_spinlock)); - mask = vm_info->channel_mask; - ITERATIVE_BITMASK_CHECK_64(mask, i) { +memcpy (mask, (char *)vm_info->channel_mask,POWER_MGR_MAX_CPUS ); +for (i =0 ; i <POWER_MGR_MAX_CPUS ; i++) { +if (mask[i] !=1 ) +continue ; vm_info->channels[i]->status = status; num_channels_changed++; }
@@ -665,8 +675,7 @@ get_all_vm(int *num_vm, int *num_vcpu)virNodeInfo node_info;virDomainPtr *domptr; -uint64_t mask; -int i, ii, numVcpus[MAX_VCPUS ], cpu, n_vcpus; +int i, ii, numVcpus[MAX_VCPUS ], n_vcpus;unsigned int jj;const char *vm_name; unsigned int domain_flags = VIR_CONNECT_LIST_DOMAINS_RUNNING |
@@ -714,16 +723,12 @@ get_all_vm(int *num_vm, int *num_vcpu)/* Save pcpu in use by libvirt VMs */ for (ii =0 ; ii < n_vcpus; ii++) { - mask =0 ;for (jj =0 ; jj < global_n_host_cpus; jj++) {if (VIR_CPU_USABLE (global_cpumaps, global_maplen, ii, jj) >0 ) { - mask |=1ULL << jj; + lvm_info[i].pcpus[ii] = jj; } } - ITERATIVE_BITMASK_CHECK_64(mask, cpu) { - lvm_info[i].pcpus[ii] = cpu; - } } } }
@@ -733,7 +738,7 @@ get_info_vm(const char *vm_name, struct vm_info *info) {struct virtual_machine_info *vm_info;unsigned i, channel_num =0 ; -uint64_t mask; +char mask[POWER_MGR_MAX_CPUS ]; vm_info =find_domain_by_name (vm_name);if (vm_info ==NULL ) {
@@ -746,13 +751,18 @@ get_info_vm(const char *vm_name, struct vm_info *info)rte_spinlock_lock (&(vm_info->config_spinlock)); - mask = vm_info->channel_mask; - ITERATIVE_BITMASK_CHECK_64(mask, i) { +memcpy (mask, (char *)vm_info->channel_mask,POWER_MGR_MAX_CPUS ); +for (i =0 ; i <POWER_MGR_MAX_CPUS ; i++) { +if (mask[i] !=1 ) +continue ; info->channels[channel_num].channel_num = i;memcpy (info->channels[channel_num].channel_path, - vm_info->channels[i]->channel_path,UNIX_PATH_MAX ); - info->channels[channel_num].status = vm_info->channels[i]->status; - info->channels[channel_num].fd = vm_info->channels[i]->fd; + vm_info->channels[i]->channel_path, +UNIX_PATH_MAX ); + info->channels[channel_num].status = + vm_info->channels[i]->status; + info->channels[channel_num].fd = + vm_info->channels[i]->fd; channel_num++; }
@@ -822,7 +832,7 @@ add_vm(const char *vm_name) }strncpy (new_domain->name, vm_name,sizeof (new_domain->name)); new_domain->name[sizeof (new_domain->name) -1 ] ='\0' ; - new_domain->channel_mask =0 ; +memset (new_domain->channel_mask,0 ,POWER_MGR_MAX_CPUS ); new_domain->num_channels =0 ;if (!virDomainIsActive (dom_ptr))
@@ -940,16 +950,19 @@ voidchannel_manager_exit (void) {unsigned i; -uint64_t mask; +char mask[POWER_MGR_MAX_CPUS ];struct virtual_machine_info *vm_info; LIST_FOREACH(vm_info, &vm_list_head, vms_info) {rte_spinlock_lock (&(vm_info->config_spinlock)); - mask = vm_info->channel_mask; - ITERATIVE_BITMASK_CHECK_64(mask, i) { -remove_channel_from_monitor (vm_info->channels[i]); +memcpy (mask, (char *)vm_info->channel_mask,POWER_MGR_MAX_CPUS ); +for (i =0 ; i <POWER_MGR_MAX_CPUS ; i++) { +if (mask[i] !=1 ) +continue ; +remove_channel_from_monitor ( + vm_info->channels[i]);close (vm_info->channels[i]->fd);rte_free (vm_info->channels[i]); }
examples/vm_power_manager/channel_manager.h +1 -1
@@ -149,7 +149,7 @@ uint64_t get_pcpus_mask(struct channel_info *chan_info, unsigned vcpu);* - 0 on success. * - Negative on error. */ -int set_pcpus_mask (char *vm_name,unsigned vcpu,uint64_t core_mask); +int set_pcpus_mask (char *vm_name,unsigned int vcpu,char *core_mask);/** * Set the Physical CPU for the specified vCPU.
examples/vm_power_manager/vm_power_cli.c +3 -3
@@ -128,7 +128,7 @@ struct cmd_set_pcpu_mask_result {cmdline_fixed_string_t set_pcpu_mask;cmdline_fixed_string_t vm_name;uint8_t vcpu; -uint64_t core_mask; +char core_mask[POWER_MGR_MAX_CPUS ]; }; static void
@@ -139,10 +139,10 @@ cmd_set_pcpu_mask_parsed(void *parsed_result, struct cmdline *cl,if (set_pcpus_mask (res->vm_name, res->vcpu, res->core_mask) ==0 )cmdline_printf (cl,"Pinned vCPU(%" PRId8") to pCPU core " -"mask(0x%" PRIx64")\n" , res->vcpu, res->core_mask); +"\n" , res->vcpu);else cmdline_printf (cl,"Unable to pin vCPU(%" PRId8") to pCPU core " -"mask(0x%" PRIx64")\n" , res->vcpu, res->core_mask); +"\n" , res->vcpu); } cmdline_parse_token_string_t cmd_set_pcpu_mask =
Signature · 1 line
2.17.1
PATCH v3 2/4 examples/power: remove mask functions
In reply to David Huntsince we're moving to allowing greater than 64 cores, the mask functions that use uint64_t to perform functions on a masked set of cores are no longer needed, so removing them.
--- examples/vm_power_manager/channel_monitor.c | 24 ---- examples/vm_power_manager/power_manager.c | 68 --------- examples/vm_power_manager/vm_power_cli.c | 149 -------------------- 3 files changed, 241 deletions(-)
examples/vm_power_manager/channel_monitor.c +0 -24
@@ -681,30 +681,6 @@ process_request(struct channel_packet *pkt, struct channel_info *chan_info)default :break ; } - }else { -switch (pkt->unit) { -case (CPU_POWER_SCALE_MIN ): -power_manager_scale_mask_min (core_mask); -break ; -case (CPU_POWER_SCALE_MAX ): -power_manager_scale_mask_max (core_mask); -break ; -case (CPU_POWER_SCALE_DOWN ): -power_manager_scale_mask_down (core_mask); -break ; -case (CPU_POWER_SCALE_UP ): -power_manager_scale_mask_up (core_mask); -break ; -case (CPU_POWER_ENABLE_TURBO ): -power_manager_enable_turbo_mask (core_mask); -break ; -case (CPU_POWER_DISABLE_TURBO ): -power_manager_disable_turbo_mask (core_mask); -break ; -default : -break ; - } - } }
examples/vm_power_manager/power_manager.c +0 -68
80 lines not on this page · raw message
examples/vm_power_manager/vm_power_cli.c +0 -149
170 lines not on this page · raw message
Signature · 1 line
2.17.1
PATCH v3 3/4 examples/power: allow vms to use lcores over 63
In reply to David HuntExtending the functionality to allow vms to power manage cores beyond 63.
--- examples/vm_power_manager/channel_manager.c | 74 +++++++++------------ examples/vm_power_manager/channel_manager.h | 29 ++------ examples/vm_power_manager/channel_monitor.c | 56 ++++++---------- examples/vm_power_manager/vm_power_cli.c | 4 +- 4 files changed, 57 insertions(+), 106 deletions(-)
examples/vm_power_manager/channel_manager.c +31 -43
@@ -49,7 +49,7 @@ static bool global_hypervisor_available;*/ struct virtual_machine_info {char name[CHANNEL_MGR_MAX_NAME_LEN ]; -rte_atomic64_t pcpu_mask[CHANNEL_CMDS_MAX_CPUS ]; +uint16_t pcpu_map[CHANNEL_CMDS_MAX_CPUS ];struct channel_info *channels[CHANNEL_CMDS_MAX_VM_CHANNELS ];char channel_mask[POWER_MGR_MAX_CPUS ];uint8_t num_channels;
@@ -79,7 +79,6 @@ update_pcpus_mask(struct virtual_machine_info *vm_info)virVcpuInfoPtr cpuinfo;unsigned i, j;int n_vcpus; -uint64_t mask;memset (global_cpumaps,0 ,CHANNEL_CMDS_MAX_CPUS *global_maplen);
@@ -119,27 +118,24 @@ update_pcpus_mask(struct virtual_machine_info *vm_info) n_vcpus); vm_info->info.nrVirtCpu = n_vcpus; } +rte_spinlock_lock (&(vm_info->config_spinlock));for (i =0 ; i < vm_info->info.nrVirtCpu; i++) { - mask =0 ;for (j =0 ; j < global_n_host_cpus; j++) { -if (VIR_CPU_USABLE (global_cpumaps, global_maplen, i, j) >0 ) { - mask |=1ULL << j; - } +if (VIR_CPU_USABLE (global_cpumaps, + global_maplen, i, j) <=0 ) +continue ; + vm_info->pcpu_map[i] = j; } -rte_atomic64_set (&vm_info->pcpu_mask[i], mask); } +rte_spinlock_unlock (&(vm_info->config_spinlock));return 0 ; }int -set_pcpus_mask (char *vm_name,unsigned int vcpu,char *core_mask) +set_pcpu (char *vm_name,unsigned int vcpu,unsigned int pcpu) { -unsigned i =0 ;int flags =VIR_DOMAIN_AFFECT_LIVE |VIR_DOMAIN_AFFECT_CONFIG ;struct virtual_machine_info *vm_info; -char mask[POWER_MGR_MAX_CPUS ]; - -memcpy (mask, core_mask,POWER_MGR_MAX_CPUS );if (vcpu >=CHANNEL_CMDS_MAX_CPUS ) { RTE_LOG(ERR, CHANNEL_MANAGER, "vCPU(%u) exceeds max allowable(%d)\n",
@@ -166,17 +162,16 @@ set_pcpus_mask(char *vm_name, unsigned int vcpu, char *core_mask)return -1 ; }memset (global_cpumaps,0 ,CHANNEL_CMDS_MAX_CPUS * global_maplen); -for (i =0 ; i <POWER_MGR_MAX_CPUS ; i++) { -if (mask[i] !=1 ) -continue ; -VIR_USE_CPU (global_cpumaps, i); -if (i >= global_n_host_cpus) { -RTE_LOG (ERR ,CHANNEL_MANAGER ,"CPU(%u) exceeds the available " -"number of CPUs(%u)\n" , - i, global_n_host_cpus); -return -1 ; - } + +VIR_USE_CPU (global_cpumaps, pcpu); + +if (pcpu >= global_n_host_cpus) { +RTE_LOG (ERR ,CHANNEL_MANAGER ,"CPU(%u) exceeds the available " +"number of CPUs(%u)\n" , + pcpu, global_n_host_cpus); +return -1 ; } +if (virDomainPinVcpuFlags (vm_info->domainPtr, vcpu, global_cpumaps, global_maplen, flags) <0 ) { RTE_LOG(ERR, CHANNEL_MANAGER, "Unable to set vCPU(%u) to pCPU "
@@ -185,33 +180,24 @@ set_pcpus_mask(char *vm_name, unsigned int vcpu, char *core_mask)return -1 ; }rte_spinlock_lock (&(vm_info->config_spinlock)); -memcpy (&vm_info->pcpu_mask[vcpu], mask,POWER_MGR_MAX_CPUS ); + vm_info->pcpu_map[vcpu] = pcpu;rte_spinlock_unlock (&(vm_info->config_spinlock));return 0 ; - -} - -int -set_pcpu (char *vm_name,unsigned vcpu,unsigned core_num) -{ -char mask[POWER_MGR_MAX_CPUS ]; - -memset (mask,0 ,POWER_MGR_MAX_CPUS ); - - mask[core_num] =1 ; - -return set_pcpus_mask (vm_name, vcpu, mask); } -uint64_t -get_pcpus_mask (struct channel_info *chan_info,unsigned vcpu) +uint16_t +get_pcpu (struct channel_info *chan_info,unsigned int vcpu) {struct virtual_machine_info *vm_info = (struct virtual_machine_info *)chan_info->priv_info; -if (global_hypervisor_available && (vm_info !=NULL )) -return rte_atomic64_read (&vm_info->pcpu_mask[vcpu]); -else +if (global_hypervisor_available && (vm_info !=NULL )) { +uint16_t pcpu; +rte_spinlock_lock (&(vm_info->config_spinlock)); + pcpu = vm_info->pcpu_map[vcpu]; +rte_spinlock_unlock (&(vm_info->config_spinlock)); +return pcpu; + }else return 0 ; }
@@ -771,9 +757,11 @@ get_info_vm(const char *vm_name, struct vm_info *info)rte_spinlock_unlock (&(vm_info->config_spinlock));memcpy (info->name, vm_info->name,sizeof (vm_info->name)); +rte_spinlock_lock (&(vm_info->config_spinlock));for (i =0 ; i < info->num_vcpus; i++) { - info->pcpu_mask[i] =rte_atomic64_read (&vm_info->pcpu_mask[i]); + info->pcpu_map[i] = vm_info->pcpu_map[i]; } +rte_spinlock_unlock (&(vm_info->config_spinlock));return 0 ; }
@@ -823,7 +811,7 @@ add_vm(const char *vm_name) }for (i =0 ; i <CHANNEL_CMDS_MAX_CPUS ; i++) { -rte_atomic64_init (&new_domain->pcpu_mask[i]); + new_domain->pcpu_map[i] =0 ; }if (update_pcpus_mask (new_domain) <0 ) {RTE_LOG (ERR ,CHANNEL_MANAGER ,"Error getting physical CPU pinning\n" );
examples/vm_power_manager/channel_manager.h +5 -24
@@ -82,7 +82,7 @@ struct channel_info {struct vm_info {char name[CHANNEL_MGR_MAX_NAME_LEN ];/**< VM name */ enum vm_status status;/**< libvirt status */ -uint64_t pcpu_mask[CHANNEL_CMDS_MAX_CPUS ];/**< pCPU mask for each vCPU */ +uint16_t pcpu_map[CHANNEL_CMDS_MAX_CPUS ];/**< pCPU map to vCPU */ unsigned num_vcpus;/**< number of vCPUS */ struct channel_info channels[CHANNEL_MGR_MAX_CHANNELS ];/**< Array of channel_info */ unsigned num_channels;/**< Number of channels */
@@ -115,8 +115,7 @@ int channel_manager_init(const char *path);void channel_manager_exit (void );/** -* Get the Physical CPU mask for VM lcore channel(vcpu), result is assigned to -* core_mask. +* Get the Physical CPU for VM lcore channel(vcpu). * It is not thread-safe. * * @param chan_info
@@ -130,26 +129,7 @@ void channel_manager_exit(void);* - 0 on error. * - >0 on success. */ -uint64_t get_pcpus_mask (struct channel_info *chan_info,unsigned vcpu); - -/** -* Set the Physical CPU mask for the specified vCPU. -* It is not thread-safe. -* -* @param name -* Virtual Machine name to lookup -* -* @param vcpu -* The virtual CPU to set. -* -* @param core_mask -* The core mask of the physical CPU(s) to bind the vCPU -* -* @return -* - 0 on success. -* - Negative on error. -*/ -int set_pcpus_mask (char *vm_name,unsigned int vcpu,char *core_mask); +uint16_t get_pcpu (struct channel_info *chan_info,unsigned int vcpu);/** * Set the Physical CPU for the specified vCPU.
@@ -168,7 +148,8 @@ int set_pcpus_mask(char *vm_name, unsigned int vcpu, char *core_mask);* - 0 on success. * - Negative on error. */ -int set_pcpu (char *vm_name,unsigned vcpu,unsigned core_num); +int set_pcpu (char *vm_name,unsigned int vcpu,unsigned int pcpu); +/** * Add a VM as specified by name to the Channel Manager. The name must * correspond to a valid libvirt domain name.
examples/vm_power_manager/channel_monitor.c +19 -37
88 lines not on this page · raw message
examples/vm_power_manager/vm_power_cli.c +2 -2
@@ -95,8 +95,8 @@ cmd_show_vm_parsed(void *parsed_result, struct cmdline *cl, }cmdline_printf (cl,"Virtual CPU(s): %u\n" , info.num_vcpus);for (i =0 ; i < info.num_vcpus; i++) { -cmdline_printf (cl," [%u]: Physical CPU Mask 0x%" PRIx64"\n" , i, - info.pcpu_mask[i]); +cmdline_printf (cl," [%u]: Physical CPU %d\n" , i, + info.pcpu_map[i]); } }
Signature · 1 line
2.17.1
PATCH v3 4/4 examples/power: increase max cores to 256
In reply to David HuntIncrease the number of addressable cores from 64 to 256. Also remove the warning that incresing this number beyond 64 will cause problems (because of the previous use of uint64_t masks). Now this number can be increased significantly without causing problems.
--- examples/vm_power_manager/channel_manager.h | 8 ++------ examples/vm_power_manager/power_manager.h | 2 +- 2 files changed, 3 insertions(+), 7 deletions(-)
examples/vm_power_manager/channel_manager.h +2 -6
@@ -14,17 +14,13 @@ extern "C" {#include <rte_atomic.h> /* Maximum number of CPUs */ -#define CHANNEL_CMDS_MAX_CPUS 64 -#if CHANNEL_CMDS_MAX_CPUS >64 -#error Maximum number of cores is 64, overflow is guaranteed to \ - cause problems with VM Power Management -#endif +#define CHANNEL_CMDS_MAX_CPUS 256/* Maximum name length including '\0' terminator */ #define CHANNEL_MGR_MAX_NAME_LEN 64/* Maximum number of channels to each Virtual Machine */ -#define CHANNEL_MGR_MAX_CHANNELS 64 +#define CHANNEL_MGR_MAX_CHANNELS 256/* Hypervisor Path for libvirt(qemu/KVM) */ #define CHANNEL_MGR_DEFAULT_HV_PATH "qemu:///system"
examples/vm_power_manager/power_manager.h +1 -1
@@ -33,7 +33,7 @@ core_info_init(void);#define RTE_LOGTYPE_POWER_MANAGER RTE_LOGTYPE_USER1/* Maximum number of CPUS to manage */ -#define POWER_MGR_MAX_CPUS 64 +#define POWER_MGR_MAX_CPUS 256/** * Initialize power management. * Initializes resources and verifies the number of CPUs on the system.
Signature · 1 line
2.17.1
Quoted 12 lines · David Hunt
First of all we convert all the relevant uint64_t's to char arrays. Then we remove the unneeded mask functions that were limited to 64 cores. Also extend the guest functionality and finally rause the number of supported cores to something more sensible, i.e. 256.
v2 changes:
* Updates out of Review by Anatoly.
* Indentation fixes
* resolved missing spinlock around config updates
* simplified some loops
* fixed bug in non-equivalent code
Quoted 10 lines
v3 changes:
* Moved a spinlock to a more appropriate place. Was in a for-loop, made more sense to have it outside.[1/4] examples/power: change 64-bit masks to arrays [2/4] examples/power: remove mask functions [3/4] examples/power: allow vms to use lcores over 63 [4/4] examples/power: increase max cores to 256
Signature · 2 lines
Thanks, Anatoly
Thomas Monjalon
Applied14/12/2018 14:37, Burakov, Anatoly:
Quoted 17 lines · David Hunt
Quoted 16 lines
First of all we convert all the relevant uint64_t's to char arrays. Then we remove the unneeded mask functions that were limited to 64 cores. Also extend the guest functionality and finally rause the number of supported cores to something more sensible, i.e. 256.
v2 changes:
* Updates out of Review by Anatoly.
* Indentation fixes
* resolved missing spinlock around config updates
* simplified some loops
* fixed bug in non-equivalent codev3 changes:
* Moved a spinlock to a more appropriate place. Was in a for-loop, made more sense to have it outside.
Quoted 6 lines
Quoted 4 lines
[1/4] examples/power: change 64-bit masks to arrays [2/4] examples/power: remove mask functions [3/4] examples/power: allow vms to use lcores over 63 [4/4] examples/power: increase max cores to 256
Reviewed-by Anatoly Burakov <[email protected]>
Applied, thanks
PATCH 2/4 examples/power: remove mask functions
In reply to David Huntsince we're moving to allowing greater than 64 cores, the mask functions that use uint64_t to perform functions on a masked set of cores are no longer feasable, so removing them.
--- examples/vm_power_manager/channel_monitor.c | 24 ---- examples/vm_power_manager/power_manager.c | 68 --------- examples/vm_power_manager/vm_power_cli.c | 149 -------------------- 3 files changed, 241 deletions(-)
examples/vm_power_manager/channel_monitor.c +0 -24
@@ -681,30 +681,6 @@ process_request(struct channel_packet *pkt, struct channel_info *chan_info)default :break ; } - }else { -switch (pkt->unit) { -case (CPU_POWER_SCALE_MIN ): -power_manager_scale_mask_min (core_mask); -break ; -case (CPU_POWER_SCALE_MAX ): -power_manager_scale_mask_max (core_mask); -break ; -case (CPU_POWER_SCALE_DOWN ): -power_manager_scale_mask_down (core_mask); -break ; -case (CPU_POWER_SCALE_UP ): -power_manager_scale_mask_up (core_mask); -break ; -case (CPU_POWER_ENABLE_TURBO ): -power_manager_enable_turbo_mask (core_mask); -break ; -case (CPU_POWER_DISABLE_TURBO ): -power_manager_disable_turbo_mask (core_mask); -break ; -default : -break ; - } - } }
examples/vm_power_manager/power_manager.c +0 -68
80 lines not on this page · raw message
examples/vm_power_manager/vm_power_cli.c +0 -149
170 lines not on this page · raw message
Signature · 1 line
2.17.1
Re: [PATCH v1 2/4] examples/power: remove mask functions
Quoted 3 lines · David Hunt
since we're moving to allowing greater than 64 cores, the mask functions that use
uint64_tto perform functions on a masked set of cores are no longer feasable, so removing them.
Perhaps "needed" is a better word, rather than "feasible" :)
Please correct me if i'm wrong, but as of this patch, some of the functionality is left in half-working state, and this patch should really be merged with patch 3?
Signature · 2 lines
Thanks, Anatoly
Re: [PATCH v1 2/4] examples/power: remove mask functions
Hi Anatoly,
Quoted 6 lines · Anatoly
Quoted 3 lines · David Hunt
since we're moving to allowing greater than 64 cores, the mask functions that use
uint64_tto perform functions on a masked set of cores are no longer feasable, so removing them.Perhaps "needed" is a better word, rather than "feasible" :)
Yes, "needed" is probably better. :)
Quoted 3 lines
Please correct me if i'm wrong, but as of this patch, some of the functionality is left in half-working state, and this patch should really be merged with patch 3?
This patch removes all the mask functions, including the CLI option to call them, so is in a fully working state. I think it should be fine as a standalone patch in the series.
Thanks,
Dave.
Burakov, Anatoly
Reviewed-byRe: [PATCH v1 2/4] examples/power: remove mask functions
Quoted 11 lines · David
Hi Anatoly,
Quoted 6 lines · Anatoly
Quoted 3 lines · David Hunt
since we're moving to allowing greater than 64 cores, the mask functions that use
uint64_tto perform functions on a masked set of cores are no longer feasable, so removing them.Perhaps "needed" is a better word, rather than "feasible" :)
Quoted 8 lines
Yes, "needed" is probably better. :)
Quoted 3 lines
Please correct me if i'm wrong, but as of this patch, some of the functionality is left in half-working state, and this patch should really be merged with patch 3?
This patch removes all the mask functions, including the CLI option to call them, so is in a fully working state. I think it should be fine as a standalone patch in the series.
Yep, OK.
(provided you fix the commit message...)
Quoted 6 lines
Thanks,
Dave.
Signature · 2 lines
Thanks, Anatoly
PATCH 3/4 examples/power: allow vms to use lcores over 63
In reply to David HuntExtending the functionality to allow vms to power manage cores beyond 63.
--- examples/vm_power_manager/channel_manager.c | 59 ++++++++------------- examples/vm_power_manager/channel_manager.h | 30 ++--------- examples/vm_power_manager/channel_monitor.c | 56 +++++++------------ examples/vm_power_manager/vm_power_cli.c | 4 +- 4 files changed, 48 insertions(+), 101 deletions(-)
examples/vm_power_manager/channel_manager.c +22 -37
@@ -49,7 +49,7 @@ static bool global_hypervisor_available;*/ struct virtual_machine_info {char name[CHANNEL_MGR_MAX_NAME_LEN ]; -rte_atomic64_t pcpu_mask[CHANNEL_CMDS_MAX_CPUS ]; +uint16_t pcpu_map[CHANNEL_CMDS_MAX_CPUS ];struct channel_info *channels[CHANNEL_CMDS_MAX_VM_CHANNELS ];char channel_mask[POWER_MGR_MAX_CPUS ];uint8_t num_channels;
@@ -79,7 +79,7 @@ update_pcpus_mask(struct virtual_machine_info *vm_info)virVcpuInfoPtr cpuinfo;unsigned i, j;int n_vcpus; -uint64_t mask; +uint16_t pcpu;memset (global_cpumaps,0 ,CHANNEL_CMDS_MAX_CPUS *global_maplen);
@@ -120,26 +120,23 @@ update_pcpus_mask(struct virtual_machine_info *vm_info) vm_info->info.nrVirtCpu = n_vcpus; }for (i =0 ; i < vm_info->info.nrVirtCpu; i++) { - mask =0 ; + pcpu =0 ;for (j =0 ; j < global_n_host_cpus; j++) {if (VIR_CPU_USABLE (global_cpumaps, global_maplen, i, j) >0 ) { - mask |=1ULL << j; + pcpu = j; } } -rte_atomic64_set (&vm_info->pcpu_mask[i], mask); + vm_info->pcpu_map[i] = pcpu; }return 0 ; }int -set_pcpus_mask (char *vm_name,unsigned int vcpu,char *core_mask) +set_pcpu (char *vm_name,unsigned int vcpu,unsigned int pcpu) {unsigned i =0 ;int flags =VIR_DOMAIN_AFFECT_LIVE |VIR_DOMAIN_AFFECT_CONFIG ;struct virtual_machine_info *vm_info; -char mask[POWER_MGR_MAX_CPUS ]; - -memcpy (mask, core_mask,POWER_MGR_MAX_CPUS );if (vcpu >=CHANNEL_CMDS_MAX_CPUS ) { RTE_LOG(ERR, CHANNEL_MANAGER, "vCPU(%u) exceeds max allowable(%d)\n",
@@ -166,17 +163,16 @@ set_pcpus_mask(char *vm_name, unsigned int vcpu, char *core_mask)return -1 ; }memset (global_cpumaps,0 ,CHANNEL_CMDS_MAX_CPUS * global_maplen); -for (i =0 ; i <POWER_MGR_MAX_CPUS ; i++) { -if (mask[i] ==1 ) { -VIR_USE_CPU (global_cpumaps, i); -if (i >= global_n_host_cpus) { -RTE_LOG (ERR ,CHANNEL_MANAGER ,"CPU(%u) exceeds the available " -"number of CPUs(%u)\n" , - i, global_n_host_cpus); -return -1 ; - } - } + +VIR_USE_CPU (global_cpumaps, i); + +if (pcpu >= global_n_host_cpus) { +RTE_LOG (ERR ,CHANNEL_MANAGER ,"CPU(%u) exceeds the available " +"number of CPUs(%u)\n" , + i, global_n_host_cpus); +return -1 ; } +if (virDomainPinVcpuFlags (vm_info->domainPtr, vcpu, global_cpumaps, global_maplen, flags) <0 ) { RTE_LOG(ERR, CHANNEL_MANAGER, "Unable to set vCPU(%u) to pCPU "
@@ -184,31 +180,20 @@ set_pcpus_mask(char *vm_name, unsigned int vcpu, char *core_mask) vm_info->name);return -1 ; } -memcpy (&vm_info->pcpu_mask[vcpu], core_mask,POWER_MGR_MAX_CPUS ); -return 0 ; - -} - -int -set_pcpu (char *vm_name,unsigned vcpu,unsigned core_num) -{ -char mask[POWER_MGR_MAX_CPUS ]; -memset (mask,0 ,POWER_MGR_MAX_CPUS ); + vm_info->pcpu_map[vcpu] = pcpu; - mask[core_num] =1 ; - -return set_pcpus_mask (vm_name, vcpu, mask); +return 0 ; } -uint64_t -get_pcpus_mask (struct channel_info *chan_info,unsigned vcpu) +uint16_t +get_pcpu (struct channel_info *chan_info,unsigned int vcpu) {struct virtual_machine_info *vm_info = (struct virtual_machine_info *)chan_info->priv_info;if (global_hypervisor_available && (vm_info !=NULL )) -return rte_atomic64_read (&vm_info->pcpu_mask[vcpu]); +return vm_info->pcpu_map[vcpu];else return 0 ; }
@@ -776,7 +761,7 @@ get_info_vm(const char *vm_name, struct vm_info *info)memcpy (info->name, vm_info->name,sizeof (vm_info->name));for (i =0 ; i < info->num_vcpus; i++) { - info->pcpu_mask[i] =rte_atomic64_read (&vm_info->pcpu_mask[i]); + info->pcpu_map[i] = vm_info->pcpu_map[i]; }return 0 ; }
@@ -827,7 +812,7 @@ add_vm(const char *vm_name) }for (i =0 ; i <CHANNEL_CMDS_MAX_CPUS ; i++) { -rte_atomic64_init (&new_domain->pcpu_mask[i]); + new_domain->pcpu_map[i] =0 ; }if (update_pcpus_mask (new_domain) <0 ) {RTE_LOG (ERR ,CHANNEL_MANAGER ,"Error getting physical CPU pinning\n" );
examples/vm_power_manager/channel_manager.h +5 -25
@@ -40,7 +40,6 @@ struct sockaddr_un _sockaddr_un;#define MAX_CLIENTS 64#define MAX_VCPUS 20 -struct libvirt_vm_info {const char *vm_name;unsigned int pcpus[MAX_VCPUS ];
@@ -82,7 +81,7 @@ struct channel_info {struct vm_info {char name[CHANNEL_MGR_MAX_NAME_LEN ];/**< VM name */ enum vm_status status;/**< libvirt status */ -uint64_t pcpu_mask[CHANNEL_CMDS_MAX_CPUS ];/**< pCPU mask for each vCPU */ +uint16_t pcpu_map[CHANNEL_CMDS_MAX_CPUS ];/**< pCPU map to vCPU */ unsigned num_vcpus;/**< number of vCPUS */ struct channel_info channels[CHANNEL_MGR_MAX_CHANNELS ];/**< Array of channel_info */ unsigned num_channels;/**< Number of channels */
@@ -115,8 +114,7 @@ int channel_manager_init(const char *path);void channel_manager_exit (void );/** -* Get the Physical CPU mask for VM lcore channel(vcpu), result is assigned to -* core_mask. +* Get the Physical CPU for VM lcore channel(vcpu). * It is not thread-safe. * * @param chan_info
@@ -130,26 +128,7 @@ void channel_manager_exit(void);* - 0 on error. * - >0 on success. */ -uint64_t get_pcpus_mask (struct channel_info *chan_info,unsigned vcpu); - -/** -* Set the Physical CPU mask for the specified vCPU. -* It is not thread-safe. -* -* @param name -* Virtual Machine name to lookup -* -* @param vcpu -* The virtual CPU to set. -* -* @param core_mask -* The core mask of the physical CPU(s) to bind the vCPU -* -* @return -* - 0 on success. -* - Negative on error. -*/ -int set_pcpus_mask (char *vm_name,unsigned int vcpu,char *core_mask); +uint16_t get_pcpu (struct channel_info *chan_info,unsigned int vcpu);/** * Set the Physical CPU for the specified vCPU.
@@ -168,7 +147,8 @@ int set_pcpus_mask(char *vm_name, unsigned int vcpu, char *core_mask);* - 0 on success. * - Negative on error. */ -int set_pcpu (char *vm_name,unsigned vcpu,unsigned core_num); +int set_pcpu (char *vm_name,unsigned int vcpu,unsigned int pcpu); +/** * Add a VM as specified by name to the Channel Manager. The name must * correspond to a valid libvirt domain name.
examples/vm_power_manager/channel_monitor.c +19 -37
88 lines not on this page · raw message
examples/vm_power_manager/vm_power_cli.c +2 -2
@@ -95,8 +95,8 @@ cmd_show_vm_parsed(void *parsed_result, struct cmdline *cl, }cmdline_printf (cl,"Virtual CPU(s): %u\n" , info.num_vcpus);for (i =0 ; i < info.num_vcpus; i++) { -cmdline_printf (cl," [%u]: Physical CPU Mask 0x%" PRIx64"\n" , i, - info.pcpu_mask[i]); +cmdline_printf (cl," [%u]: Physical CPU %d\n" , i, + info.pcpu_map[i]); } }
Signature · 1 line
2.17.1
Re: [PATCH v1 3/4] examples/power: allow vms to use lcores over 63
Quoted 33 lines · David Hunt
Extending the functionality to allow vms to power manage cores beyond 63.
Signed-off-by David Hunt <[email protected]>--- examples/vm_power_manager/channel_manager.c | 59 ++++++++------------- examples/vm_power_manager/channel_manager.h | 30 ++--------- examples/vm_power_manager/channel_monitor.c | 56 +++++++------------ examples/vm_power_manager/vm_power_cli.c | 4 +- 4 files changed, 48 insertions(+), 101 deletions(-)examples/vm_power_manager/channel_manager.c +2
-2@@ -49,7 +49,7 @@ static bool global_hypervisor_available;*/ struct virtual_machine_info {char name[CHANNEL_MGR_MAX_NAME_LEN ]; -rte_atomic64_t pcpu_mask[CHANNEL_CMDS_MAX_CPUS ]; +uint16_t pcpu_map[CHANNEL_CMDS_MAX_CPUS ];struct channel_info *channels[CHANNEL_CMDS_MAX_VM_CHANNELS ];char channel_mask[POWER_MGR_MAX_CPUS ];uint8_t num_channels;@@ -79,7 +79,7 @@ update_pcpus_mask(struct virtual_machine_info *vm_info)virVcpuInfoPtr cpuinfo;unsigned i, j;int n_vcpus; -uint64_t mask; +uint16_t pcpu;memset (global_cpumaps,0 ,CHANNEL_CMDS_MAX_CPUS *global_maplen);
examples/vm_power_manager/channel_manager.c update_pcpus_mask(struct virtual_machine_info *vm_info) · line 126
Quoted 9 lines
examples/vm_power_manager/channel_manager.c +2
-2@@ -120,26 +120,23 @@ update_pcpus_mask(struct virtual_machine_info *vm_info) vm_info->info.nrVirtCpu = n_vcpus; }for (i =0 ; i < vm_info->info.nrVirtCpu; i++) { - mask =0 ; + pcpu =0 ;for (j =0 ; j < global_n_host_cpus; j++) {if (VIR_CPU_USABLE (global_cpumaps, global_maplen, i, j) >0 ) { - mask |=1ULL << j; + pcpu = j;
Not sure what this does.
Initial code goes through all CPU's, marks all that are usable into mask (i.e. code implies there can be several), and then sets up this mask.
Now, you're going through all CPU's, store *one* arbitrary CPU index, and set the map up with that single index.
That doesn't look like an equivalent replacement.
examples/vm_power_manager/channel_manager.c update_pcpus_mask(struct virtual_machine_info *vm_info) · line 135
Quoted 11 lines
examples/vm_power_manager/channel_manager.c +2
-2} } -rte_atomic64_set (&vm_info->pcpu_mask[i], mask); + vm_info->pcpu_map[i] = pcpu; }return 0 ; }int -set_pcpus_mask (char *vm_name,unsigned int vcpu,char *core_mask) +set_pcpu (char *vm_name,unsigned int vcpu,unsigned int pcpu)
See patch 1 comments - it would've made this change easier to parse if set_pcpu was removed there, instead of here.
Quoted 23 lines
examples/vm_power_manager/channel_manager.c +0
-12{unsigned i =0 ;int flags =VIR_DOMAIN_AFFECT_LIVE |VIR_DOMAIN_AFFECT_CONFIG ;struct virtual_machine_info *vm_info; -char mask[POWER_MGR_MAX_CPUS ]; - -memcpy (mask, core_mask,POWER_MGR_MAX_CPUS );if (vcpu >=CHANNEL_CMDS_MAX_CPUS ) { RTE_LOG(ERR, CHANNEL_MANAGER, "vCPU(%u) exceeds max allowable(%d)\n",@@ -166,17 +163,16 @@ set_pcpus_mask(char *vm_name, unsigned int vcpu, char *core_mask)return -1 ; }memset (global_cpumaps,0 ,CHANNEL_CMDS_MAX_CPUS * global_maplen); -for (i =0 ; i <POWER_MGR_MAX_CPUS ; i++) { -if (mask[i] ==1 ) { -VIR_USE_CPU (global_cpumaps, i); -if (i >= global_n_host_cpus) { -RTE_LOG (ERR ,CHANNEL_MANAGER ,"CPU(%u) exceeds the available " -"number of CPUs(%u)\n" , - i, global_n_host_cpus); -return -1 ; - }
examples/vm_power_manager/channel_manager.c set_pcpus_mask(char *vm_name, unsigned int vcpu, char *core_mask) · line 174
Quoted 10 lines
examples/vm_power_manager/channel_manager.c +8
-1- } + +VIR_USE_CPU (global_cpumaps, i); + +if (pcpu >= global_n_host_cpus) { +RTE_LOG (ERR ,CHANNEL_MANAGER ,"CPU(%u) exceeds the available " +"number of CPUs(%u)\n" , + i, global_n_host_cpus); +return -1 ; }
Some comments on what the above code does would have been nice. Why the check was removed?
Quoted 1 line
examples/vm_power_manager/channel_manager.c +1
-0+
Signature · 2 lines
Thanks, Anatoly
Re: [PATCH v1 3/4] examples/power: allow vms to use lcores over 63
Hi Anatoly,
Quoted 44 lines · Anatoly
Quoted 39 lines · David Hunt
Extending the functionality to allow vms to power manage cores beyond 63.
Signed-off-by David Hunt <[email protected]>---
examples/vm_power_manager/channel_manager.c | 59 ++++++++------------- examples/vm_power_manager/channel_manager.h | 30 ++--------- examples/vm_power_manager/channel_monitor.c | 56 +++++++------------ examples/vm_power_manager/vm_power_cli.c | 4 +- 4 files changed, 48 insertions(+), 101 deletions(-)examples/vm_power_manager/channel_manager.c +2
-2@@ -49,7 +49,7 @@ static bool global_hypervisor_available;*/ struct virtual_machine_info { char name[CHANNEL_MGR_MAX_NAME_LEN]; - rte_atomic64_t pcpu_mask[CHANNEL_CMDS_MAX_CPUS]; + uint16_t pcpu_map[CHANNEL_CMDS_MAX_CPUS]; struct channel_info *channels[CHANNEL_CMDS_MAX_VM_CHANNELS]; char channel_mask[POWER_MGR_MAX_CPUS]; uint8_t num_channels;@@ -79,7 +79,7 @@ update_pcpus_mask(struct virtual_machine_info *vm_info) virVcpuInfoPtr cpuinfo; unsigned i, j; int n_vcpus; - uint64_t mask; + uint16_t pcpu; memset(global_cpumaps, 0, CHANNEL_CMDS_MAX_CPUS*global_maplen); @@ -120,26 +120,23 @@ update_pcpus_mask(struct virtual_machine_info *vm_info)
vm_info->info.nrVirtCpu=n_vcpus;
}
for (i = 0; i <vm_info->info.nrVirtCpu; i++) {
- mask = 0;
+ pcpu = 0;
for (j = 0; j <global_n_host_cpus; j++) {
if (VIR_CPU_USABLE(global_cpumaps, global_maplen, i, j)Quoted 3 lines
Quoted 1 line
0) {
- mask |= 1ULL << j;
+ pcpu = j;
Quoted 8 lines
Not sure what this does.
Initial code goes through all CPU's, marks all that are usable into mask (i.e. code implies there can be several), and then sets up this mask.
Now, you're going through all CPU's, store *one* arbitrary CPU index, and set the map up with that single index.
That doesn't look like an equivalent replacement.
Good catch. mask was changed to an array, so I need to set element in the loop, not outside it. Fixed in next rev.
Quoted 4 lines
Quoted 4 lines
}
}
-rte_atomic64_set(&vm_info->pcpu_mask[i], mask); + vm_info->pcpu_map[i] = pcpu;
Quoted 9 lines
examples/vm_power_manager/channel_manager.c update_pcpus_mask(struct virtual_machine_info *vm_info) · line 135
Quoted 6 lines
}
return 0;
}
intexamples/vm_power_manager/channel_manager.c +1
-1-set_pcpus_mask(char *vm_name, unsigned int vcpu, char *core_mask) +set_pcpu(char *vm_name, unsigned int vcpu, unsigned int pcpu)See patch 1 comments - it would've made this change easier to parse if
set_pcpuwas removed there, instead of here.
I couldn't change it back there, as the calling functions had not been updated. Now that they are gone, I can remove it in this patch.
Quoted 28 lines
Quoted 28 lines
{
unsigned i = 0;
int flags = VIR_DOMAIN_AFFECT_LIVE|VIR_DOMAIN_AFFECT_CONFIG; structvirtual_machine_info*vm_info;
- char mask[POWER_MGR_MAX_CPUS];
-
-memcpy(mask, core_mask, POWER_MGR_MAX_CPUS);
if (vcpu >=CHANNEL_CMDS_MAX_CPUS) {
RTE_LOG(ERR,CHANNEL_MANAGER, "vCPU(%u) exceeds max allowable(%d)\n",examples/vm_power_manager/channel_manager.c +0
-5@@ -166,17 +163,16 @@ set_pcpus_mask(char *vm_name, unsigned int vcpu, char *core_mask) return -1; } memset(global_cpumaps, 0 , CHANNEL_CMDS_MAX_CPUS * global_maplen); - for (i = 0; i < POWER_MGR_MAX_CPUS; i++) { - if (mask[i] == 1) { - VIR_USE_CPU(global_cpumaps, i); - if (i >= global_n_host_cpus) { - RTE_LOG(ERR, CHANNEL_MANAGER, "CPU(%u) exceeds theavailable "
- "number of CPUs(%u)\n",
- i,global_n_host_cpus);
- return -1;
- }
- }examples/vm_power_manager/channel_manager.c +3
-0+ + VIR_USE_CPU(global_cpumaps, i); +
Quoted 8 lines
Quoted 6 lines
+ if (pcpu >=
global_n_host_cpus) {
+ RTE_LOG(ERR,CHANNEL_MANAGER, "CPU(%u) exceeds the available " + "number of CPUs(%u)\n",
+ i,global_n_host_cpus);
+ return -1;
}Some comments on what the above code does would have been nice. Why the check was removed?
function changed from set_pcpus_mask to the name of what used to be the wrapper function, and in doing so we changed the mask parameter to a pcpu (int) parameter, so there's not need for a loop any more. However, there was an error where I was calling VIR_USE_CPU with the incorrect value, so that's now fixed. The check was removed because we're no longer checking for '1's in an array, we are being passed the actual pcpu to use.
Also, there were an addition of a spinlock around reading the pcpu config data.
Quoted 3 lines
Quoted 2 lines
+
PATCH 4/4 examples/power: increase MAX_CPUS to 256
In reply to David HuntIncrease the number of addressable cores from 64 to 256. Also remove the warning that incresing this number beyond 64 will cause problems (because of the previous use of uint64_t masks). Now this number can be increased significantly without causing problems.
--- examples/vm_power_manager/channel_manager.h | 8 ++------ examples/vm_power_manager/power_manager.h | 2 +- 2 files changed, 3 insertions(+), 7 deletions(-)
examples/vm_power_manager/channel_manager.h +2 -6
@@ -14,17 +14,13 @@ extern "C" {#include <rte_atomic.h> /* Maximum number of CPUs */ -#define CHANNEL_CMDS_MAX_CPUS 64 -#if CHANNEL_CMDS_MAX_CPUS >64 -#error Maximum number of cores is 64, overflow is guaranteed to \ - cause problems with VM Power Management -#endif +#define CHANNEL_CMDS_MAX_CPUS 256/* Maximum name length including '\0' terminator */ #define CHANNEL_MGR_MAX_NAME_LEN 64/* Maximum number of channels to each Virtual Machine */ -#define CHANNEL_MGR_MAX_CHANNELS 64 +#define CHANNEL_MGR_MAX_CHANNELS 256/* Hypervisor Path for libvirt(qemu/KVM) */ #define CHANNEL_MGR_DEFAULT_HV_PATH "qemu:///system"
examples/vm_power_manager/power_manager.h +1 -1
@@ -33,7 +33,7 @@ core_info_init(void);#define RTE_LOGTYPE_POWER_MANAGER RTE_LOGTYPE_USER1/* Maximum number of CPUS to manage */ -#define POWER_MGR_MAX_CPUS 64 +#define POWER_MGR_MAX_CPUS 256/** * Initialize power management. * Initializes resources and verifies the number of CPUs on the system.
Signature · 1 line
2.17.1
Re: [PATCH v1 4/4] examples/power: increase MAX_CPUS to 256
Quoted 7 lines · David Hunt
Increase the number of addressable cores from 64 to 256. Also remove the warning that incresing this number beyond 64 will cause problems (because of the previous use of
uint64_tmasks). Now this number can be increased significantly without causing problems.Signed-off-by David Hunt <[email protected]>---
Signature · 2 lines
Thanks, Anatoly