From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 0C0A81BA83 for ; Fri, 14 Dec 2018 13:29:57 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 14 Dec 2018 04:29:57 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,352,1539673200"; d="scan'208";a="118549070" Received: from dhunt5-mobl2.ger.corp.intel.com (HELO [10.237.221.66]) ([10.237.221.66]) by FMSMGA003.fm.intel.com with ESMTP; 14 Dec 2018 04:29:56 -0800 To: "Burakov, Anatoly" , dev@dpdk.org Cc: lei.a.yao@intel.com References: <20181122170220.55482-2-david.hunt@intel.com> <20181214114946.21570-1-david.hunt@intel.com> <20181214114946.21570-4-david.hunt@intel.com> <6582d776-fe33-7cf7-8715-8f919aa8d71f@intel.com> From: "Hunt, David" Message-ID: <8ca4dfd2-4719-a62a-5b43-13ecc4bae9fa@intel.com> Date: Fri, 14 Dec 2018 12:29:55 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.3.3 MIME-Version: 1.0 In-Reply-To: <6582d776-fe33-7cf7-8715-8f919aa8d71f@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US Subject: Re: [dpdk-dev] [PATCH v2 3/4] examples/power: allow vms to use lcores over 63 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 14 Dec 2018 12:29:58 -0000 Hi Anatoly, On 14/12/2018 12:08 PM, Burakov, Anatoly wrote: > On 14-Dec-18 11:49 AM, David Hunt wrote: >> Extending the functionality to allow vms to power manage cores beyond >> 63. >> >> Signed-off-by: David Hunt >> --- >>   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(-) >> >> diff --git a/examples/vm_power_manager/channel_manager.c >> b/examples/vm_power_manager/channel_manager.c >> index 71f4a0ccf..8756b53b8 100644 >> --- a/examples/vm_power_manager/channel_manager.c >> +++ b/examples/vm_power_manager/channel_manager.c >> @@ -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)); > > 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. >>           } >> -        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) >>   { > > > >> --- a/examples/vm_power_manager/channel_manager.h >> +++ b/examples/vm_power_manager/channel_manager.h >> @@ -40,7 +40,6 @@ struct sockaddr_un _sockaddr_un; >>   #define MAX_CLIENTS 64 >>   #define MAX_VCPUS 20 >>   - >>   struct libvirt_vm_info { > > Unintended whitespace change? > Yes, will address. >>       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 */ > Thanks, Dave.