diff mbox series

[v3,13/19] vhost: register new regions with userfaultfd

Message ID	20181004081403.8039-14-maxime.coquelin@redhat.com (mailing list archive)
State	Superseded, archived
Delegated to:	Maxime Coquelin
Headers	From: Maxime Coquelin <maxime.coquelin@redhat.com> To: dev@dpdk.org, tiwei.bie@intel.com, zhihong.wang@intel.com, jfreimann@redhat.com, nicknickolaev@gmail.com, i.maximets@samsung.com, bruce.richardson@intel.com, alejandro.lucero@netronome.com Cc: dgilbert@redhat.com, stable@dpdk.org, Maxime Coquelin <maxime.coquelin@redhat.com> Date: Thu, 4 Oct 2018 10:13:57 +0200 Message-Id: <20181004081403.8039-14-maxime.coquelin@redhat.com> In-Reply-To: <20181004081403.8039-1-maxime.coquelin@redhat.com> References: <20181004081403.8039-1-maxime.coquelin@redhat.com> Subject: [dpdk-dev] [PATCH v3 13/19] vhost: register new regions with userfaultfd Precedence: list Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org>
Series	vhost: add postcopy live-migration support \| [v3,00/19] vhost: add postcopy live-migration support [v3,01/19] vhost: fix messages error checks [v3,02/19] vhost: fix return code of messages requiring replies [v3,03/19] vhost: clarify reply-ack in case a reply was already sent [v3,04/19] vhost: fix payload size of reply [v3,05/19] vhost: fix error handling when mem table gets updated [v3,06/19] vhost: define postcopy protocol flag [v3,07/19] vhost: add number of fds to vhost-user messages and use it [v3,08/19] vhost: pass socket fd to message handling callbacks [v3,09/19] vhost: enable fds passing when sending vhost-user messages [v3,10/19] vhost: add config flag for postcopy feature [v3,11/19] vhost: introduce postcopy's advise message [v3,12/19] vhost: add support for postcopy's listen message [v3,13/19] vhost: register new regions with userfaultfd [v3,14/19] vhost: avoid useless VhostUserMemory copy [v3,15/19] vhost: send userfault range addresses back to qemu [v3,16/19] vhost: add support to postcopy's end request [v3,17/19] vhost: enable postcopy protocol feature [v3,18/19] vhost: add flag to enable postcopy live-migration [v3,19/19] net/vhost: add parameter to enable postcopy support

Checks

Context	Check	Description
ci/Intel-compilation	success	Compilation OK

Commit Message

Maxime Coquelin Oct. 4, 2018, 8:13 a.m. UTC

  Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost_user.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

Comments

Ilya Maximets Oct. 5, 2018, 12:34 p.m. UTC | #1

On 04.10.2018 11:13, Maxime Coquelin wrote:
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  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,
> +						&reg_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().

>  err_mmap:
>  	free_mem_region(dev);
>  	rte_free(dev->mem);
>

Maxime Coquelin Oct. 8, 2018, 1:03 p.m. UTC | #2

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 <dgilbert@redhat.com>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>>   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,
>> +						&reg_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);
>>

diff mbox series

Patch

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,
+						&reg_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;
+	}
 err_mmap:
 	free_mem_region(dev);
 	rte_free(dev->mem);