Re: [RFC PATCH v4 17/17] virtio/vsock: simplify credit update function API

From: Stefano Garzarella
Date: Thu Feb 11 2021 - 10:02:01 EST


On Sun, Feb 07, 2021 at 06:19:03PM +0300, Arseny Krasnov wrote:
'virtio_transport_send_credit_update()' has some extra args:
1) 'type' may be set in 'virtio_transport_send_pkt_info()' using type
of socket.
2) This function is static and 'hdr' arg was always NULL.


Okay, I saw this patch after my previous comment.

I think this looks good, but please move this before your changes (e.g. before patch 'virtio/vsock: dequeue callback for SOCK_SEQPACKET').

In this way you don't need to modify virtio_transport_notify_buffer_size(), calling virtio_transport_get_type() and then remove these changes.

It's generally not a good idea to make changes in a patch and then remove them a few patches later in the same series. This should ring a bell about moving these changes before others.

Thanks,
Stefano

Signed-off-by: Arseny Krasnov <arseny.krasnov@xxxxxxxxxxxxx>
---
net/vmw_vsock/virtio_transport_common.c | 20 +++++---------------
1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 0aa0fd33e9d6..46308679c8a4 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -286,13 +286,10 @@ void virtio_transport_put_credit(struct virtio_vsock_sock *vvs, u32 credit)
}
EXPORT_SYMBOL_GPL(virtio_transport_put_credit);

-static int virtio_transport_send_credit_update(struct vsock_sock *vsk,
- int type,
- struct virtio_vsock_hdr *hdr)
+static int virtio_transport_send_credit_update(struct vsock_sock *vsk)
{
struct virtio_vsock_pkt_info info = {
.op = VIRTIO_VSOCK_OP_CREDIT_UPDATE,
- .type = type,
.vsk = vsk,
};

@@ -401,9 +398,7 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
* with different values.
*/
if (free_space < VIRTIO_VSOCK_MAX_PKT_BUF_SIZE) {
- virtio_transport_send_credit_update(vsk,
- VIRTIO_VSOCK_TYPE_STREAM,
- NULL);
+ virtio_transport_send_credit_update(vsk);
}

return total;
@@ -525,9 +520,7 @@ size_t virtio_transport_seqpacket_seq_get_len(struct vsock_sock *vsk)
spin_unlock_bh(&vvs->rx_lock);

if (bytes_dropped)
- virtio_transport_send_credit_update(vsk,
- VIRTIO_VSOCK_TYPE_SEQPACKET,
- NULL);
+ virtio_transport_send_credit_update(vsk);

return vvs->user_read_seq_len;
}
@@ -624,8 +617,7 @@ static int virtio_transport_seqpacket_do_dequeue(struct vsock_sock *vsk,

spin_unlock_bh(&vvs->rx_lock);

- virtio_transport_send_credit_update(vsk, VIRTIO_VSOCK_TYPE_SEQPACKET,
- NULL);
+ virtio_transport_send_credit_update(vsk);

return err;
}
@@ -735,15 +727,13 @@ EXPORT_SYMBOL_GPL(virtio_transport_do_socket_init);
void virtio_transport_notify_buffer_size(struct vsock_sock *vsk, u64 *val)
{
struct virtio_vsock_sock *vvs = vsk->trans;
- int type;

if (*val > VIRTIO_VSOCK_MAX_BUF_SIZE)
*val = VIRTIO_VSOCK_MAX_BUF_SIZE;

vvs->buf_alloc = *val;

- type = virtio_transport_get_type(sk_vsock(vsk));
- virtio_transport_send_credit_update(vsk, type, NULL);
+ virtio_transport_send_credit_update(vsk);
}
EXPORT_SYMBOL_GPL(virtio_transport_notify_buffer_size);

--
2.25.1