From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id 0B0652B9A; Mon, 17 Sep 2018 12:18:42 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 17 Sep 2018 03:18:42 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.53,385,1531810800"; d="scan'208";a="84156417" Received: from aburakov-mobl1.ger.corp.intel.com (HELO [10.237.220.55]) ([10.237.220.55]) by orsmga003.jf.intel.com with ESMTP; 17 Sep 2018 03:18:39 -0700 To: Tiwei Bie Cc: maxime.coquelin@redhat.com, zhihong.wang@intel.com, dev@dpdk.org, seanbh@gmail.com, stable@dpdk.org References: <20180905042852.6212-1-tiwei.bie@intel.com> <20180905042852.6212-4-tiwei.bie@intel.com> <20180907113744.GA22511@debian> <20180910040441.GB23854@debian> From: "Burakov, Anatoly" Message-ID: Date: Mon, 17 Sep 2018 11:18:38 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20180910040441.GB23854@debian> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 3/3] net/virtio-user: fix memory hotplug support in vhost-kernel 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: Mon, 17 Sep 2018 10:18:43 -0000 On 10-Sep-18 5:04 AM, Tiwei Bie wrote: > On Fri, Sep 07, 2018 at 01:24:05PM +0100, Burakov, Anatoly wrote: >> On 07-Sep-18 12:37 PM, Tiwei Bie wrote: >>> On Fri, Sep 07, 2018 at 10:44:22AM +0100, Burakov, Anatoly wrote: >>>> On 05-Sep-18 5:28 AM, Tiwei Bie wrote: >>>>> It's possible to have much more hugepage backed memory regions >>>>> than what vhost-kernel supports due to the memory hotplug, which >>>>> may cause problems. A better solution is to have the virtio-user >>>>> pass all the memory ranges reserved by DPDK to vhost-kernel. >>>>> >>>>> Fixes: 12ecb2f63b12 ("net/virtio-user: support memory hotplug") >>>>> Cc: stable@dpdk.org >>>>> >>>>> Signed-off-by: Tiwei Bie >>>>> --- >>>>> drivers/net/virtio/virtio_user/vhost_kernel.c | 38 +++++++++---------- >>>>> 1 file changed, 18 insertions(+), 20 deletions(-) >>>>> >>>>> diff --git a/drivers/net/virtio/virtio_user/vhost_kernel.c b/drivers/net/virtio/virtio_user/vhost_kernel.c >>>>> index 897fee0af..9338166d9 100644 >>>>> --- a/drivers/net/virtio/virtio_user/vhost_kernel.c >>>>> +++ b/drivers/net/virtio/virtio_user/vhost_kernel.c >>>>> @@ -70,41 +70,41 @@ static uint64_t vhost_req_user_to_kernel[] = { >>>>> [VHOST_USER_SET_MEM_TABLE] = VHOST_SET_MEM_TABLE, >>>>> }; >>>>> -struct walk_arg { >>>>> - struct vhost_memory_kernel *vm; >>>>> - uint32_t region_nr; >>>>> -}; >>>>> static int >>>>> -add_memory_region(const struct rte_memseg_list *msl __rte_unused, >>>>> - const struct rte_memseg *ms, size_t len, void *arg) >>>>> +add_memseg_list(const struct rte_memseg_list *msl, void *arg) >>>>> { >>>>> - struct walk_arg *wa = arg; >>>>> + struct vhost_memory_kernel *vm = arg; >>>>> struct vhost_memory_region *mr; >>>>> void *start_addr; >>>>> + uint64_t len; >>>>> - if (wa->region_nr >= max_regions) >>>>> + if (vm->nregions >= max_regions) >>>>> return -1; >>>>> - mr = &wa->vm->regions[wa->region_nr++]; >>>>> - start_addr = ms->addr; >>>>> + start_addr = msl->base_va; >>>>> + len = msl->page_sz * msl->memseg_arr.len; >>>>> + >>>>> + mr = &vm->regions[vm->nregions++]; >>>>> mr->guest_phys_addr = (uint64_t)(uintptr_t)start_addr; >>>>> mr->userspace_addr = (uint64_t)(uintptr_t)start_addr; >>>>> mr->memory_size = len; >>>>> - mr->mmap_offset = 0; >>>>> + mr->mmap_offset = 0; /* flags_padding */ >>>>> + >>>>> + PMD_DRV_LOG(DEBUG, "index=%u addr=%p len=%" PRIu64, >>>>> + vm->nregions - 1, start_addr, len); >>>>> return 0; >>>>> } >>>>> -/* By default, vhost kernel module allows 64 regions, but DPDK allows >>>>> - * 256 segments. As a relief, below function merges those virtually >>>>> - * adjacent memsegs into one region. >>>>> +/* By default, vhost kernel module allows 64 regions, but DPDK may >>>>> + * have much more memory regions. Below function will treat each >>>>> + * contiguous memory space reserved by DPDK as one region. >>>>> */ >>>>> static struct vhost_memory_kernel * >>>>> prepare_vhost_memory_kernel(void) >>>>> { >>>>> struct vhost_memory_kernel *vm; >>>>> - struct walk_arg wa; >>>>> vm = malloc(sizeof(struct vhost_memory_kernel) + >>>>> max_regions * >>>>> @@ -112,20 +112,18 @@ prepare_vhost_memory_kernel(void) >>>>> if (!vm) >>>>> return NULL; >>>>> - wa.region_nr = 0; >>>>> - wa.vm = vm; >>>>> + vm->nregions = 0; >>>>> + vm->padding = 0; >>>>> /* >>>>> * The memory lock has already been taken by memory subsystem >>>>> * or virtio_user_start_device(). >>>>> */ >>>>> - if (rte_memseg_contig_walk_thread_unsafe(add_memory_region, &wa) < 0) { >>>>> + if (rte_memseg_list_walk_thread_unsafe(add_memseg_list, vm) < 0) { >>>>> free(vm); >>>>> return NULL; >>>>> } >>>>> - vm->nregions = wa.region_nr; >>>>> - vm->padding = 0; >>>>> return vm; >>>>> } >>>>> >>>> >>>> Doesn't that assume single file segments mode? >>> >>> This is to find out the VA ranges reserved by memory subsystem. >>> Why does it need to assume single file segments mode? >> >> If you are not in single-file segments mode, each individual page in a >> VA-contiguous area will be behind a different fd - so it will be part of a >> different region, would it not? > > Above code is for vhost-kernel. Kernel doesn't need the > fds to get the access to virtio-user process's memory. > Kernel just needs to know the mappings between GPA (guest > physical address) and VA (virtio-user's virtual address). > Ah OK. Thanks for clarification! -- Thanks, Anatoly