From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id 87453DED for ; Tue, 28 Aug 2018 10:44:22 +0200 (CEST) X-Amp-Result: UNSCANNABLE X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 28 Aug 2018 01:44:21 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.53,298,1531810800"; d="scan'208";a="85091522" Received: from fbsd.sh.intel.com ([10.67.104.231]) by fmsmga001.fm.intel.com with ESMTP; 28 Aug 2018 01:44:20 -0700 Date: Tue, 28 Aug 2018 16:42:03 +0800 From: Tiwei Bie To: "Burakov, Anatoly" Cc: maxime.coquelin@redhat.com, zhihong.wang@intel.com, dev@dpdk.org Message-ID: <20180828084203.GA69008@fbsd.sh.intel.com> References: <20180828075327.18842-1-tiwei.bie@intel.com> <986b3012-552b-6673-d716-9acacb5e9528@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <986b3012-552b-6673-d716-9acacb5e9528@intel.com> User-Agent: Mutt/1.10.1 (2018-07-13) Subject: Re: [dpdk-dev] [RFC] net/virtio-user: avoid parsing process mappings 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: Tue, 28 Aug 2018 08:44:23 -0000 On Tue, Aug 28, 2018 at 09:12:38AM +0100, Burakov, Anatoly wrote: > On 28-Aug-18 8:53 AM, Tiwei Bie wrote: [...] > > > > - for (i = 0; i < num; ++i) { > > - mr = &msg->payload.memory.regions[i]; > > - mr->guest_phys_addr = huges[i].addr; /* use vaddr! */ > > - mr->userspace_addr = huges[i].addr; > > - mr->memory_size = huges[i].size; > > - mr->mmap_offset = 0; > > - fds[i] = open(huges[i].path, O_RDWR); > > + start_addr = (uint64_t)(uintptr_t)ms->addr; > > + end_addr = start_addr + ms->len; > > + > > + /* > > + * XXX workaround! > > + * > > + * For --single-file-segments case, offset should be: > > + * offset = rte_fbarray_find_idx(&msl->memseg_arr, ms) * msl->page_sz; > > + * > > + * But it's not true for non-single-file-segments case. > > + * > > + * This is a temporary workaround which assumes the file will > > + * also be mapped from the beginning in --single-file-segments > > + * case. It should be replaced when we have a memory API to > > + * get the offset. > > Yes, this is an unfortunate consequence of having split personalities in > memory subsystem. A good solution would be to just deprecate > non-single-file segments mode and always use it, but we cannot do that > because we support kernel versions that do not support fallocate() on > hugetlbfs (and it still leaves legacy mem mode, which effectively > behaves like non-single-file segments as far as virtio is concerned). > > How about we add this API in v2 for seg fd patchset? Sure, thanks! Besides, I saw another minor issue. When --legacy-mem and --single-file-segments are both specified, the EAL init doesn't fail, but fd can't be got successfully. > > > + */ > > + offset = 0; > > + [...] > > @@ -109,9 +111,24 @@ is_vhost_user_by_type(const char *path) > > int > > virtio_user_start_device(struct virtio_user_dev *dev) > > { > > + struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config; > > uint64_t features; > > int ret; > > > > + /* > > + * XXX workaround! > > + * > > + * We need to make sure that the locks will be > > + * taken in the correct order to avoid deadlocks. > > + * > > + * Before releasing this lock, this thread should > > + * not trigger any memory hotplug events. > > + * > > + * This is a temporary workaround, and should be > > + * replaced when we get proper supports from the > > + * memory subsystem in the future. > > + */ > > + rte_rwlock_read_lock(&mcfg->memory_hotplug_lock); > > This feels like it should be a separate patch, because it fixes the > issue that exists independently of whether we have segment fd API or not. Yeah, I'll put it in a separate patch. > > > pthread_mutex_lock(&dev->mutex); > > > > if (is_vhost_user_by_type(dev->path) && dev->vhostfd < 0) > > @@ -152,10 +169,12 @@ virtio_user_start_device(struct virtio_user_dev *dev) > > > > dev->started = true; > > pthread_mutex_unlock(&dev->mutex); > > + rte_rwlock_read_unlock(&mcfg->memory_hotplug_lock); > > > > return 0; > > error: > > pthread_mutex_unlock(&dev->mutex); > > + rte_rwlock_read_unlock(&mcfg->memory_hotplug_lock); > > /* TODO: free resource here or caller to check */ > > return -1; > > } > > > > > -- > Thanks, > Anatoly