From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <david.hunt@intel.com>
Received: from mga04.intel.com (mga04.intel.com [192.55.52.120])
by dpdk.org (Postfix) with ESMTP id 0C0A81BA83
for <dev@dpdk.org>; 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" <anatoly.burakov@intel.com>, 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" <david.hunt@intel.com>
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=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 <david.hunt@intel.com>
>> ---
>> 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)
>> {
>
> <snip>
>
>> --- 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.