| Message ID | 20190531133954.122567-3-sgarzare@redhat.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | vsock/virtio: optimizations to increase the throughput | expand |
From: Stefano Garzarella <sgarzare@redhat.com> Date: Fri, 31 May 2019 15:39:51 +0200 > @@ -434,7 +434,9 @@ void virtio_transport_set_buffer_size(struct vsock_sock *vsk, u64 val) > if (val > vvs->buf_size_max) > vvs->buf_size_max = val; > vvs->buf_size = val; > + spin_lock_bh(&vvs->rx_lock); > vvs->buf_alloc = val; > + spin_unlock_bh(&vvs->rx_lock); This locking doesn't do anything other than to strongly order the buf_size store to occur before the buf_alloc one. If you need a memory barrier, use one.
On Sun, Jun 02, 2019 at 06:03:34PM -0700, David Miller wrote: > From: Stefano Garzarella <sgarzare@redhat.com> > Date: Fri, 31 May 2019 15:39:51 +0200 > > > @@ -434,7 +434,9 @@ void virtio_transport_set_buffer_size(struct vsock_sock *vsk, u64 val) > > if (val > vvs->buf_size_max) > > vvs->buf_size_max = val; > > vvs->buf_size = val; > > + spin_lock_bh(&vvs->rx_lock); > > vvs->buf_alloc = val; > > + spin_unlock_bh(&vvs->rx_lock); > > This locking doesn't do anything other than to strongly order the > buf_size store to occur before the buf_alloc one. Sure, I'll remove the lock. I was confused because I moved its reading under the rx_lock (together with other variables), but here I'm updating only buf_alloc, so this lock is useless. Thanks, Stefano
diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h index 7d973903f52e..53703fe03681 100644 --- a/include/linux/virtio_vsock.h +++ b/include/linux/virtio_vsock.h @@ -35,11 +35,11 @@ struct virtio_vsock_sock { /* Protected by tx_lock */ u32 tx_cnt; - u32 buf_alloc; u32 peer_fwd_cnt; u32 peer_buf_alloc; /* Protected by rx_lock */ + u32 buf_alloc; u32 fwd_cnt; u32 rx_bytes; struct list_head rx_queue; diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c index 4fd4987511a9..694d9805f989 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c @@ -211,10 +211,10 @@ static void virtio_transport_dec_rx_pkt(struct virtio_vsock_sock *vvs, void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct virtio_vsock_pkt *pkt) { - spin_lock_bh(&vvs->tx_lock); + spin_lock_bh(&vvs->rx_lock); pkt->hdr.fwd_cnt = cpu_to_le32(vvs->fwd_cnt); pkt->hdr.buf_alloc = cpu_to_le32(vvs->buf_alloc); - spin_unlock_bh(&vvs->tx_lock); + spin_unlock_bh(&vvs->rx_lock); } EXPORT_SYMBOL_GPL(virtio_transport_inc_tx_pkt); @@ -434,7 +434,9 @@ void virtio_transport_set_buffer_size(struct vsock_sock *vsk, u64 val) if (val > vvs->buf_size_max) vvs->buf_size_max = val; vvs->buf_size = val; + spin_lock_bh(&vvs->rx_lock); vvs->buf_alloc = val; + spin_unlock_bh(&vvs->rx_lock); } EXPORT_SYMBOL_GPL(virtio_transport_set_buffer_size);
fwd_cnt is written with rx_lock, so we should read it using the same spinlock also if we are in the TX path. Move also buf_alloc under rx_lock and add a missing locking when we modify it. Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> --- include/linux/virtio_vsock.h | 2 +- net/vmw_vsock/virtio_transport_common.c | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-)