On Fri, Apr 05, 2019 at 10:16:48AM +0200, Stefano Garzarella wrote: > On Thu, Apr 04, 2019 at 08:15:39PM +0100, Stefan Hajnoczi wrote: > > On Thu, Apr 04, 2019 at 12:58:35PM +0200, Stefano Garzarella wrote: > > > int err = -EFAULT; > > > > > > spin_lock_bh(&vvs->rx_lock); > > > @@ -288,9 +290,15 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk, > > > } > > > spin_unlock_bh(&vvs->rx_lock); > > > > > > - /* Send a credit pkt to peer */ > > > - virtio_transport_send_credit_update(vsk, VIRTIO_VSOCK_TYPE_STREAM, > > > - NULL); > > > + /* We send a credit update only when the space available seen > > > + * by the transmitter is less than VIRTIO_VSOCK_MAX_PKT_BUF_SIZE > > > + */ > > > + free_space = vvs->buf_alloc - (vvs->fwd_cnt - vvs->last_fwd_cnt); > > > > Locking? These fields should be accessed under tx_lock. > > > > Yes, we need a lock, but looking in the code, vvs->fwd_cnd is written > taking rx_lock (virtio_transport_dec_rx_pkt) and it is read with the > tx_lock (virtio_transport_inc_tx_pkt). > > Maybe we should use another spin_lock shared between RX and TX for those > fields or use atomic variables. > > What do you suggest? Or make vvs->fwd_cnt atomic if it's the only field that needs to be accessed in this manner. Stefan
Attachment:
signature.asc
Description: PGP signature
