Re: [PATCH RFC v5 12/13] vhost/vsock: switch to the buf API

2020-06-08 Thread Stefano Garzarella
On Mon, Jun 08, 2020 at 09:30:38AM -0400, Michael S. Tsirkin wrote:
> On Mon, Jun 08, 2020 at 12:17:46PM +0200, Stefano Garzarella wrote:
> > On Sun, Jun 07, 2020 at 10:11:49AM -0400, Michael S. Tsirkin wrote:
> > > A straight-forward conversion.
> > > 
> > > Signed-off-by: Michael S. Tsirkin 
> > > ---
> > >  drivers/vhost/vsock.c | 30 ++
> > >  1 file changed, 18 insertions(+), 12 deletions(-)
> > 
> > The changes for vsock part LGTM:
> > 
> > Reviewed-by: Stefano Garzarella 
> > 
> > 
> > I also did some tests with vhost-vsock (tools/testing/vsock/vsock_test
> > and iperf-vsock), so for vsock:
> > 
> > Tested-by: Stefano Garzarella 
> > 
> > Thanks,
> > Stefano
> 
> Re-testing v6 would be very much appreciated.

Sure, I'm building v6 now and I'll send you a feedback :-)

Stefano

> 
> > > 
> > > diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> > > index a483cec31d5c..61c6d3dd2ae3 100644
> > > --- a/drivers/vhost/vsock.c
> > > +++ b/drivers/vhost/vsock.c
> > > @@ -103,7 +103,8 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
> > >   unsigned out, in;
> > >   size_t nbytes;
> > >   size_t iov_len, payload_len;
> > > - int head;
> > > + struct vhost_buf buf;
> > > + int ret;
> > >  
> > >   spin_lock_bh(>send_pkt_list_lock);
> > >   if (list_empty(>send_pkt_list)) {
> > > @@ -117,16 +118,17 @@ vhost_transport_do_send_pkt(struct vhost_vsock 
> > > *vsock,
> > >   list_del_init(>list);
> > >   spin_unlock_bh(>send_pkt_list_lock);
> > >  
> > > - head = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
> > > -  , , NULL, NULL);
> > > - if (head < 0) {
> > > + ret = vhost_get_avail_buf(vq, ,
> > > +   vq->iov, ARRAY_SIZE(vq->iov),
> > > +   , , NULL, NULL);
> > > + if (ret < 0) {
> > >   spin_lock_bh(>send_pkt_list_lock);
> > >   list_add(>list, >send_pkt_list);
> > >   spin_unlock_bh(>send_pkt_list_lock);
> > >   break;
> > >   }
> > >  
> > > - if (head == vq->num) {
> > > + if (!ret) {
> > >   spin_lock_bh(>send_pkt_list_lock);
> > >   list_add(>list, >send_pkt_list);
> > >   spin_unlock_bh(>send_pkt_list_lock);
> > > @@ -186,7 +188,8 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
> > >*/
> > >   virtio_transport_deliver_tap_pkt(pkt);
> > >  
> > > - vhost_add_used(vq, head, sizeof(pkt->hdr) + payload_len);
> > > + buf.in_len = sizeof(pkt->hdr) + payload_len;
> > > + vhost_put_used_buf(vq, );
> > >   added = true;
> > >  
> > >   pkt->off += payload_len;
> > > @@ -440,7 +443,8 @@ static void vhost_vsock_handle_tx_kick(struct 
> > > vhost_work *work)
> > >   struct vhost_vsock *vsock = container_of(vq->dev, struct vhost_vsock,
> > >dev);
> > >   struct virtio_vsock_pkt *pkt;
> > > - int head, pkts = 0, total_len = 0;
> > > + int ret, pkts = 0, total_len = 0;
> > > + struct vhost_buf buf;
> > >   unsigned int out, in;
> > >   bool added = false;
> > >  
> > > @@ -461,12 +465,13 @@ static void vhost_vsock_handle_tx_kick(struct 
> > > vhost_work *work)
> > >   goto no_more_replies;
> > >   }
> > >  
> > > - head = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
> > > -  , , NULL, NULL);
> > > - if (head < 0)
> > > + ret = vhost_get_avail_buf(vq, ,
> > > +   vq->iov, ARRAY_SIZE(vq->iov),
> > > +   , , NULL, NULL);
> > > + if (ret < 0)
> > >   break;
> > >  
> > > - if (head == vq->num) {
> > > + if (!ret) {
> > >   if (unlikely(vhost_enable_notify(>dev, vq))) {
> > >   vhost_disable_notify(>dev, vq);
> > >   continue;
> > > @@ -494,7 +499,8 @@ static void vhost_vsock_handle_tx_kick(struct 
> > > vhost_work *work)
> > >   virtio_transport_free_pkt(pkt);
> > >  
> > >   len += sizeof(pkt->hdr);
> > > - vhost_add_used(vq, head, len);
> > > + buf.in_len = len;
> > > + vhost_put_used_buf(vq, );
> > >   total_len += len;
> > >   added = true;
> > >   } while(likely(!vhost_exceeds_weight(vq, ++pkts, total_len)));
> > > -- 
> > > MST
> > > 
> 



Re: [PATCH RFC v5 12/13] vhost/vsock: switch to the buf API

2020-06-08 Thread Michael S. Tsirkin
On Mon, Jun 08, 2020 at 12:17:46PM +0200, Stefano Garzarella wrote:
> On Sun, Jun 07, 2020 at 10:11:49AM -0400, Michael S. Tsirkin wrote:
> > A straight-forward conversion.
> > 
> > Signed-off-by: Michael S. Tsirkin 
> > ---
> >  drivers/vhost/vsock.c | 30 ++
> >  1 file changed, 18 insertions(+), 12 deletions(-)
> 
> The changes for vsock part LGTM:
> 
> Reviewed-by: Stefano Garzarella 
> 
> 
> I also did some tests with vhost-vsock (tools/testing/vsock/vsock_test
> and iperf-vsock), so for vsock:
> 
> Tested-by: Stefano Garzarella 
> 
> Thanks,
> Stefano

Re-testing v6 would be very much appreciated.

> > 
> > diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> > index a483cec31d5c..61c6d3dd2ae3 100644
> > --- a/drivers/vhost/vsock.c
> > +++ b/drivers/vhost/vsock.c
> > @@ -103,7 +103,8 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
> > unsigned out, in;
> > size_t nbytes;
> > size_t iov_len, payload_len;
> > -   int head;
> > +   struct vhost_buf buf;
> > +   int ret;
> >  
> > spin_lock_bh(>send_pkt_list_lock);
> > if (list_empty(>send_pkt_list)) {
> > @@ -117,16 +118,17 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
> > list_del_init(>list);
> > spin_unlock_bh(>send_pkt_list_lock);
> >  
> > -   head = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
> > -, , NULL, NULL);
> > -   if (head < 0) {
> > +   ret = vhost_get_avail_buf(vq, ,
> > + vq->iov, ARRAY_SIZE(vq->iov),
> > + , , NULL, NULL);
> > +   if (ret < 0) {
> > spin_lock_bh(>send_pkt_list_lock);
> > list_add(>list, >send_pkt_list);
> > spin_unlock_bh(>send_pkt_list_lock);
> > break;
> > }
> >  
> > -   if (head == vq->num) {
> > +   if (!ret) {
> > spin_lock_bh(>send_pkt_list_lock);
> > list_add(>list, >send_pkt_list);
> > spin_unlock_bh(>send_pkt_list_lock);
> > @@ -186,7 +188,8 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
> >  */
> > virtio_transport_deliver_tap_pkt(pkt);
> >  
> > -   vhost_add_used(vq, head, sizeof(pkt->hdr) + payload_len);
> > +   buf.in_len = sizeof(pkt->hdr) + payload_len;
> > +   vhost_put_used_buf(vq, );
> > added = true;
> >  
> > pkt->off += payload_len;
> > @@ -440,7 +443,8 @@ static void vhost_vsock_handle_tx_kick(struct 
> > vhost_work *work)
> > struct vhost_vsock *vsock = container_of(vq->dev, struct vhost_vsock,
> >  dev);
> > struct virtio_vsock_pkt *pkt;
> > -   int head, pkts = 0, total_len = 0;
> > +   int ret, pkts = 0, total_len = 0;
> > +   struct vhost_buf buf;
> > unsigned int out, in;
> > bool added = false;
> >  
> > @@ -461,12 +465,13 @@ static void vhost_vsock_handle_tx_kick(struct 
> > vhost_work *work)
> > goto no_more_replies;
> > }
> >  
> > -   head = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
> > -, , NULL, NULL);
> > -   if (head < 0)
> > +   ret = vhost_get_avail_buf(vq, ,
> > + vq->iov, ARRAY_SIZE(vq->iov),
> > + , , NULL, NULL);
> > +   if (ret < 0)
> > break;
> >  
> > -   if (head == vq->num) {
> > +   if (!ret) {
> > if (unlikely(vhost_enable_notify(>dev, vq))) {
> > vhost_disable_notify(>dev, vq);
> > continue;
> > @@ -494,7 +499,8 @@ static void vhost_vsock_handle_tx_kick(struct 
> > vhost_work *work)
> > virtio_transport_free_pkt(pkt);
> >  
> > len += sizeof(pkt->hdr);
> > -   vhost_add_used(vq, head, len);
> > +   buf.in_len = len;
> > +   vhost_put_used_buf(vq, );
> > total_len += len;
> > added = true;
> > } while(likely(!vhost_exceeds_weight(vq, ++pkts, total_len)));
> > -- 
> > MST
> > 



Re: [PATCH RFC v5 12/13] vhost/vsock: switch to the buf API

2020-06-08 Thread Stefano Garzarella
On Sun, Jun 07, 2020 at 10:11:49AM -0400, Michael S. Tsirkin wrote:
> A straight-forward conversion.
> 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  drivers/vhost/vsock.c | 30 ++
>  1 file changed, 18 insertions(+), 12 deletions(-)

The changes for vsock part LGTM:

Reviewed-by: Stefano Garzarella 


I also did some tests with vhost-vsock (tools/testing/vsock/vsock_test
and iperf-vsock), so for vsock:

Tested-by: Stefano Garzarella 

Thanks,
Stefano

> 
> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> index a483cec31d5c..61c6d3dd2ae3 100644
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c
> @@ -103,7 +103,8 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
>   unsigned out, in;
>   size_t nbytes;
>   size_t iov_len, payload_len;
> - int head;
> + struct vhost_buf buf;
> + int ret;
>  
>   spin_lock_bh(>send_pkt_list_lock);
>   if (list_empty(>send_pkt_list)) {
> @@ -117,16 +118,17 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
>   list_del_init(>list);
>   spin_unlock_bh(>send_pkt_list_lock);
>  
> - head = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
> -  , , NULL, NULL);
> - if (head < 0) {
> + ret = vhost_get_avail_buf(vq, ,
> +   vq->iov, ARRAY_SIZE(vq->iov),
> +   , , NULL, NULL);
> + if (ret < 0) {
>   spin_lock_bh(>send_pkt_list_lock);
>   list_add(>list, >send_pkt_list);
>   spin_unlock_bh(>send_pkt_list_lock);
>   break;
>   }
>  
> - if (head == vq->num) {
> + if (!ret) {
>   spin_lock_bh(>send_pkt_list_lock);
>   list_add(>list, >send_pkt_list);
>   spin_unlock_bh(>send_pkt_list_lock);
> @@ -186,7 +188,8 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
>*/
>   virtio_transport_deliver_tap_pkt(pkt);
>  
> - vhost_add_used(vq, head, sizeof(pkt->hdr) + payload_len);
> + buf.in_len = sizeof(pkt->hdr) + payload_len;
> + vhost_put_used_buf(vq, );
>   added = true;
>  
>   pkt->off += payload_len;
> @@ -440,7 +443,8 @@ static void vhost_vsock_handle_tx_kick(struct vhost_work 
> *work)
>   struct vhost_vsock *vsock = container_of(vq->dev, struct vhost_vsock,
>dev);
>   struct virtio_vsock_pkt *pkt;
> - int head, pkts = 0, total_len = 0;
> + int ret, pkts = 0, total_len = 0;
> + struct vhost_buf buf;
>   unsigned int out, in;
>   bool added = false;
>  
> @@ -461,12 +465,13 @@ static void vhost_vsock_handle_tx_kick(struct 
> vhost_work *work)
>   goto no_more_replies;
>   }
>  
> - head = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
> -  , , NULL, NULL);
> - if (head < 0)
> + ret = vhost_get_avail_buf(vq, ,
> +   vq->iov, ARRAY_SIZE(vq->iov),
> +   , , NULL, NULL);
> + if (ret < 0)
>   break;
>  
> - if (head == vq->num) {
> + if (!ret) {
>   if (unlikely(vhost_enable_notify(>dev, vq))) {
>   vhost_disable_notify(>dev, vq);
>   continue;
> @@ -494,7 +499,8 @@ static void vhost_vsock_handle_tx_kick(struct vhost_work 
> *work)
>   virtio_transport_free_pkt(pkt);
>  
>   len += sizeof(pkt->hdr);
> - vhost_add_used(vq, head, len);
> + buf.in_len = len;
> + vhost_put_used_buf(vq, );
>   total_len += len;
>   added = true;
>   } while(likely(!vhost_exceeds_weight(vq, ++pkts, total_len)));
> -- 
> MST
> 



Re: [PATCH RFC v5 12/13] vhost/vsock: switch to the buf API

2020-06-08 Thread Stefan Hajnoczi
On Sun, Jun 07, 2020 at 10:11:49AM -0400, Michael S. Tsirkin wrote:
> A straight-forward conversion.
> 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  drivers/vhost/vsock.c | 30 ++
>  1 file changed, 18 insertions(+), 12 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature


[PATCH RFC v5 12/13] vhost/vsock: switch to the buf API

2020-06-07 Thread Michael S. Tsirkin
A straight-forward conversion.

Signed-off-by: Michael S. Tsirkin 
---
 drivers/vhost/vsock.c | 30 ++
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index a483cec31d5c..61c6d3dd2ae3 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -103,7 +103,8 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
unsigned out, in;
size_t nbytes;
size_t iov_len, payload_len;
-   int head;
+   struct vhost_buf buf;
+   int ret;
 
spin_lock_bh(>send_pkt_list_lock);
if (list_empty(>send_pkt_list)) {
@@ -117,16 +118,17 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
list_del_init(>list);
spin_unlock_bh(>send_pkt_list_lock);
 
-   head = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
-, , NULL, NULL);
-   if (head < 0) {
+   ret = vhost_get_avail_buf(vq, ,
+ vq->iov, ARRAY_SIZE(vq->iov),
+ , , NULL, NULL);
+   if (ret < 0) {
spin_lock_bh(>send_pkt_list_lock);
list_add(>list, >send_pkt_list);
spin_unlock_bh(>send_pkt_list_lock);
break;
}
 
-   if (head == vq->num) {
+   if (!ret) {
spin_lock_bh(>send_pkt_list_lock);
list_add(>list, >send_pkt_list);
spin_unlock_bh(>send_pkt_list_lock);
@@ -186,7 +188,8 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
 */
virtio_transport_deliver_tap_pkt(pkt);
 
-   vhost_add_used(vq, head, sizeof(pkt->hdr) + payload_len);
+   buf.in_len = sizeof(pkt->hdr) + payload_len;
+   vhost_put_used_buf(vq, );
added = true;
 
pkt->off += payload_len;
@@ -440,7 +443,8 @@ static void vhost_vsock_handle_tx_kick(struct vhost_work 
*work)
struct vhost_vsock *vsock = container_of(vq->dev, struct vhost_vsock,
 dev);
struct virtio_vsock_pkt *pkt;
-   int head, pkts = 0, total_len = 0;
+   int ret, pkts = 0, total_len = 0;
+   struct vhost_buf buf;
unsigned int out, in;
bool added = false;
 
@@ -461,12 +465,13 @@ static void vhost_vsock_handle_tx_kick(struct vhost_work 
*work)
goto no_more_replies;
}
 
-   head = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
-, , NULL, NULL);
-   if (head < 0)
+   ret = vhost_get_avail_buf(vq, ,
+ vq->iov, ARRAY_SIZE(vq->iov),
+ , , NULL, NULL);
+   if (ret < 0)
break;
 
-   if (head == vq->num) {
+   if (!ret) {
if (unlikely(vhost_enable_notify(>dev, vq))) {
vhost_disable_notify(>dev, vq);
continue;
@@ -494,7 +499,8 @@ static void vhost_vsock_handle_tx_kick(struct vhost_work 
*work)
virtio_transport_free_pkt(pkt);
 
len += sizeof(pkt->hdr);
-   vhost_add_used(vq, head, len);
+   buf.in_len = len;
+   vhost_put_used_buf(vq, );
total_len += len;
added = true;
} while(likely(!vhost_exceeds_weight(vq, ++pkts, total_len)));
-- 
MST