linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] virtio_balloon: check virtqueue_add_outbuf() return value
@ 2022-05-30  2:16 Bo Liu (刘波)-浪潮信息
  2022-05-30  7:00 ` David Hildenbrand
  0 siblings, 1 reply; 6+ messages in thread
From: Bo Liu (刘波)-浪潮信息 @ 2022-05-30  2:16 UTC (permalink / raw)
  To: mst; +Cc: david, jasowang, virtualization, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 967 bytes --]

Adding this patch can avoid unnecessary VM exits and reduce the number of VM exits

-----邮件原件-----
发件人: Michael S. Tsirkin <mst@redhat.com> 
发送时间: 2022年5月27日 18:58
收件人: Bo Liu (刘波)-浪潮信息 <liubo03@inspur.com>
抄送: david@redhat.com; jasowang@redhat.com; virtualization@lists.linux-foundation.org; linux-kernel@vger.kernel.org
主题: Re: [PATCH] virtio_balloon: check virtqueue_add_outbuf() return value

On Fri, May 27, 2022 at 10:43:30AM +0000, Bo Liu (刘波)-浪潮信息 wrote:
> The purpose of this patch is, virtqueue_add_outbuf() adding data to 
> virtqueue may succeed or fail. If successful, it will call 
> virtqueue_kick() to notifie the host backend driver, but if the 
> virtqueue_add_outbuf() fails, the data is not successfully added to 
> the virtqueue, and the host back-end driver does not need to be notified for processing.
> 
> Thanks.

Why is it a problem if we do?

--
MST


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3777 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] virtio_balloon: check virtqueue_add_outbuf() return value
  2022-05-30  2:16 [PATCH] virtio_balloon: check virtqueue_add_outbuf() return value Bo Liu (刘波)-浪潮信息
@ 2022-05-30  7:00 ` David Hildenbrand
  0 siblings, 0 replies; 6+ messages in thread
From: David Hildenbrand @ 2022-05-30  7:00 UTC (permalink / raw)
  To: Bo Liu (刘波)-浪潮信息, mst
  Cc: jasowang, virtualization, linux-kernel

On 30.05.22 04:16, Bo Liu (刘波)-浪潮信息 wrote:
> Adding this patch can avoid unnecessary VM exits and reduce the number of VM exits
> 

... in corner cases where virtqueue_add_outbuf() fails? Why do we care
about that corner case?

Looks like unnecessary code churn to me, unless I am missing something
important.

-- 
Thanks,

David / dhildenb


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] virtio_balloon: check virtqueue_add_outbuf() return value
  2022-05-27 10:43 Bo Liu (刘波)-浪潮信息
@ 2022-05-27 10:57 ` Michael S. Tsirkin
  0 siblings, 0 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2022-05-27 10:57 UTC (permalink / raw)
  To: Bo Liu (刘波)-浪潮信息
  Cc: david, jasowang, virtualization, linux-kernel

On Fri, May 27, 2022 at 10:43:30AM +0000, Bo Liu (刘波)-浪潮信息 wrote:
> The purpose of this patch is, virtqueue_add_outbuf() adding data to
> virtqueue may succeed or fail. If successful, it will call virtqueue_kick()
> to notifie the host backend driver, but if the virtqueue_add_outbuf() fails,
> the data is not successfully added to the virtqueue, and the host back-end
> driver does not need to be notified for processing.
> 
> Thanks.

Why is it a problem if we do?

-- 
MST


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] virtio_balloon: check virtqueue_add_outbuf() return value
@ 2022-05-27 10:43 Bo Liu (刘波)-浪潮信息
  2022-05-27 10:57 ` Michael S. Tsirkin
  0 siblings, 1 reply; 6+ messages in thread
From: Bo Liu (刘波)-浪潮信息 @ 2022-05-27 10:43 UTC (permalink / raw)
  To: mst; +Cc: david, jasowang, virtualization, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2926 bytes --]

The purpose of this patch is, virtqueue_add_outbuf() adding data to
virtqueue may succeed or fail. If successful, it will call virtqueue_kick()
to notifie the host backend driver, but if the virtqueue_add_outbuf() fails,
the data is not successfully added to the virtqueue, and the host back-end
driver does not need to be notified for processing.

Thanks.

-----邮件原件-----
发件人: Michael S. Tsirkin <mst@redhat.com> 
发送时间: 2022年5月27日 18:25
收件人: Bo Liu (刘波)-浪潮信息 <liubo03@inspur.com>
抄送: david@redhat.com; jasowang@redhat.com;
virtualization@lists.linux-foundation.org; linux-kernel@vger.kernel.org
主题: Re: [PATCH] virtio_balloon: check virtqueue_add_outbuf() return value

On Thu, May 26, 2022 at 09:34:01PM -0400, Bo Liu wrote:
> virtqueue_add_outbuf() can fail, when it fails, there is no need to 
> call the funciont virtqueue_kick().
> Signed-off-by: Bo Liu <liubo03@inspur.com>

Sorry don't get it.
What's the point of this patch really?

> ---
>  drivers/virtio/virtio_balloon.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_balloon.c 
> b/drivers/virtio/virtio_balloon.c index b9737da6c4dd..0d7da4d95e1e 
> 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -153,16 +153,18 @@ static void tell_host(struct virtio_balloon *vb, 
> struct virtqueue *vq)  {
>  	struct scatterlist sg;
>  	unsigned int len;
> +	int err;
>  
>  	sg_init_one(&sg, vb->pfns, sizeof(vb->pfns[0]) * vb->num_pfns);
>  
>  	/* We should always be able to add one buffer to an empty queue. */
> -	virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
> -	virtqueue_kick(vq);
> -
> -	/* When host has read buffer, this completes via balloon_ack */
> -	wait_event(vb->acked, virtqueue_get_buf(vq, &len));
> +	err = virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
> +	if (!err) {
> +		virtqueue_kick(vq);
>  
> +		/* When host has read buffer, this completes via balloon_ack
*/
> +		wait_event(vb->acked, virtqueue_get_buf(vq, &len));
> +	}
>  }
>  
>  static int virtballoon_free_page_report(struct 
> page_reporting_dev_info *pr_dev_info, @@ -382,6 +384,7 @@ static void
stats_handle_request(struct virtio_balloon *vb)
>  	struct virtqueue *vq;
>  	struct scatterlist sg;
>  	unsigned int len, num_stats;
> +	int err;
>  
>  	num_stats = update_balloon_stats(vb);
>  
> @@ -389,8 +392,9 @@ static void stats_handle_request(struct virtio_balloon
*vb)
>  	if (!virtqueue_get_buf(vq, &len))
>  		return;
>  	sg_init_one(&sg, vb->stats, sizeof(vb->stats[0]) * num_stats);
> -	virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
> -	virtqueue_kick(vq);
> +	err = virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
> +	if (!err)
> +		virtqueue_kick(vq);
>  }
>  
>  static inline s64 towards_target(struct virtio_balloon *vb)
> --
> 2.27.0


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3777 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] virtio_balloon: check virtqueue_add_outbuf() return value
  2022-05-27  1:34 Bo Liu
@ 2022-05-27 10:24 ` Michael S. Tsirkin
  0 siblings, 0 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2022-05-27 10:24 UTC (permalink / raw)
  To: Bo Liu; +Cc: david, jasowang, virtualization, linux-kernel

On Thu, May 26, 2022 at 09:34:01PM -0400, Bo Liu wrote:
> virtqueue_add_outbuf() can fail, when it fails, there is no need
> to call the funciont virtqueue_kick().
> Signed-off-by: Bo Liu <liubo03@inspur.com>

Sorry don't get it.
What's the point of this patch really?

> ---
>  drivers/virtio/virtio_balloon.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index b9737da6c4dd..0d7da4d95e1e 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -153,16 +153,18 @@ static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq)
>  {
>  	struct scatterlist sg;
>  	unsigned int len;
> +	int err;
>  
>  	sg_init_one(&sg, vb->pfns, sizeof(vb->pfns[0]) * vb->num_pfns);
>  
>  	/* We should always be able to add one buffer to an empty queue. */
> -	virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
> -	virtqueue_kick(vq);
> -
> -	/* When host has read buffer, this completes via balloon_ack */
> -	wait_event(vb->acked, virtqueue_get_buf(vq, &len));
> +	err = virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
> +	if (!err) {
> +		virtqueue_kick(vq);
>  
> +		/* When host has read buffer, this completes via balloon_ack */
> +		wait_event(vb->acked, virtqueue_get_buf(vq, &len));
> +	}
>  }
>  
>  static int virtballoon_free_page_report(struct page_reporting_dev_info *pr_dev_info,
> @@ -382,6 +384,7 @@ static void stats_handle_request(struct virtio_balloon *vb)
>  	struct virtqueue *vq;
>  	struct scatterlist sg;
>  	unsigned int len, num_stats;
> +	int err;
>  
>  	num_stats = update_balloon_stats(vb);
>  
> @@ -389,8 +392,9 @@ static void stats_handle_request(struct virtio_balloon *vb)
>  	if (!virtqueue_get_buf(vq, &len))
>  		return;
>  	sg_init_one(&sg, vb->stats, sizeof(vb->stats[0]) * num_stats);
> -	virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
> -	virtqueue_kick(vq);
> +	err = virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
> +	if (!err)
> +		virtqueue_kick(vq);
>  }
>  
>  static inline s64 towards_target(struct virtio_balloon *vb)
> -- 
> 2.27.0


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH] virtio_balloon: check virtqueue_add_outbuf() return value
@ 2022-05-27  1:34 Bo Liu
  2022-05-27 10:24 ` Michael S. Tsirkin
  0 siblings, 1 reply; 6+ messages in thread
From: Bo Liu @ 2022-05-27  1:34 UTC (permalink / raw)
  To: mst, david, jasowang; +Cc: virtualization, linux-kernel, Bo Liu

virtqueue_add_outbuf() can fail, when it fails, there is no need
to call the funciont virtqueue_kick().

Signed-off-by: Bo Liu <liubo03@inspur.com>
---
 drivers/virtio/virtio_balloon.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index b9737da6c4dd..0d7da4d95e1e 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -153,16 +153,18 @@ static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq)
 {
 	struct scatterlist sg;
 	unsigned int len;
+	int err;
 
 	sg_init_one(&sg, vb->pfns, sizeof(vb->pfns[0]) * vb->num_pfns);
 
 	/* We should always be able to add one buffer to an empty queue. */
-	virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
-	virtqueue_kick(vq);
-
-	/* When host has read buffer, this completes via balloon_ack */
-	wait_event(vb->acked, virtqueue_get_buf(vq, &len));
+	err = virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
+	if (!err) {
+		virtqueue_kick(vq);
 
+		/* When host has read buffer, this completes via balloon_ack */
+		wait_event(vb->acked, virtqueue_get_buf(vq, &len));
+	}
 }
 
 static int virtballoon_free_page_report(struct page_reporting_dev_info *pr_dev_info,
@@ -382,6 +384,7 @@ static void stats_handle_request(struct virtio_balloon *vb)
 	struct virtqueue *vq;
 	struct scatterlist sg;
 	unsigned int len, num_stats;
+	int err;
 
 	num_stats = update_balloon_stats(vb);
 
@@ -389,8 +392,9 @@ static void stats_handle_request(struct virtio_balloon *vb)
 	if (!virtqueue_get_buf(vq, &len))
 		return;
 	sg_init_one(&sg, vb->stats, sizeof(vb->stats[0]) * num_stats);
-	virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
-	virtqueue_kick(vq);
+	err = virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
+	if (!err)
+		virtqueue_kick(vq);
 }
 
 static inline s64 towards_target(struct virtio_balloon *vb)
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-05-30  7:01 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-30  2:16 [PATCH] virtio_balloon: check virtqueue_add_outbuf() return value Bo Liu (刘波)-浪潮信息
2022-05-30  7:00 ` David Hildenbrand
  -- strict thread matches above, loose matches on Subject: below --
2022-05-27 10:43 Bo Liu (刘波)-浪潮信息
2022-05-27 10:57 ` Michael S. Tsirkin
2022-05-27  1:34 Bo Liu
2022-05-27 10:24 ` Michael S. Tsirkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).