From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id B69EF9B67; Mon, 8 Oct 2018 15:03:39 +0200 (CEST) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id EC4E07F6B3; Mon, 8 Oct 2018 13:03:38 +0000 (UTC) Received: from [10.36.112.51] (ovpn-112-51.ams2.redhat.com [10.36.112.51]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 28A5166079; Mon, 8 Oct 2018 13:03:32 +0000 (UTC) To: Ilya Maximets , dev@dpdk.org, tiwei.bie@intel.com, zhihong.wang@intel.com, jfreimann@redhat.com, nicknickolaev@gmail.com, bruce.richardson@intel.com, alejandro.lucero@netronome.com Cc: dgilbert@redhat.com, stable@dpdk.org References: <20181004081403.8039-1-maxime.coquelin@redhat.com> <20181004081403.8039-14-maxime.coquelin@redhat.com> <20181005123224eucas1p2eac500e48f5bb87845f8ecd6b86ff2cb~atl_eKaQ32105521055eucas1p2t@eucas1p2.samsung.com> From: Maxime Coquelin Message-ID: <7e87051b-8eef-238d-0101-4e211e34a6a5@redhat.com> Date: Mon, 8 Oct 2018 15:03:30 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20181005123224eucas1p2eac500e48f5bb87845f8ecd6b86ff2cb~atl_eKaQ32105521055eucas1p2t@eucas1p2.samsung.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Mon, 08 Oct 2018 13:03:39 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH v3 13/19] vhost: register new regions with userfaultfd 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, 08 Oct 2018 13:03:40 -0000 On 10/05/2018 02:34 PM, Ilya Maximets wrote: > On 04.10.2018 11:13, Maxime Coquelin wrote: >> Signed-off-by: Dr. David Alan Gilbert >> Signed-off-by: Maxime Coquelin >> --- >> lib/librte_vhost/vhost_user.c | 33 ++++++++++++++++++++++++++++++++- >> 1 file changed, 32 insertions(+), 1 deletion(-) >> >> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c >> index ffb3ea60a..d61be2da6 100644 >> --- a/lib/librte_vhost/vhost_user.c >> +++ b/lib/librte_vhost/vhost_user.c >> @@ -968,6 +968,32 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg, >> mmap_size, >> alignment, >> mmap_offset); >> + >> + if (dev->postcopy_listening) { >> +#ifdef RTE_LIBRTE_VHOST_POSTCOPY >> + struct uffdio_register reg_struct; >> + >> + reg_struct.range.start = (uint64_t)(uintptr_t)mmap_addr; >> + reg_struct.range.len = mmap_size; >> + reg_struct.mode = UFFDIO_REGISTER_MODE_MISSING; >> + >> + if (ioctl(dev->postcopy_ufd, UFFDIO_REGISTER, >> + ®_struct)) { >> + RTE_LOG(ERR, VHOST_CONFIG, >> + "Failed to register ufd for region %d: (ufd = %d) %s\n", >> + i, dev->postcopy_ufd, >> + strerror(errno)); >> + goto err_ufd; >> + } >> + RTE_LOG(INFO, VHOST_CONFIG, >> + "\t userfaultfd registered for range : %llx - %llx\n", >> + reg_struct.range.start, >> + reg_struct.range.start + >> + reg_struct.range.len - 1); >> +#else >> + goto err_ufd; >> +#endif >> + } >> } >> >> for (i = 0; i < dev->nr_vring; i++) { >> @@ -983,7 +1009,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg, >> >> dev = translate_ring_addresses(dev, i); >> if (!dev) >> - goto err_mmap; >> + goto err_ufd; >> >> >> *pdev = dev; >> @@ -994,6 +1020,11 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg, >> >> return VH_RESULT_OK; >> >> +err_ufd: >> + if (dev->postcopy_ufd >= 0) { >> + close(dev->postcopy_ufd); >> + dev->postcopy_ufd = -1; >> + } > > It's unclear why we need to close postcopy_ufd here because this > handler doesn't allocate it and postcopy_ufd should exist before > calling this function. > So, IMHO, we need to close it in all the VH_RESULT_ERR cases or > leave it for later vhost_backend_cleanup(). Right, I agree this is vhost_backend_cleanup()'s job. I will remove it from here. >> err_mmap: >> free_mem_region(dev); >> rte_free(dev->mem); >>