search for: virtio_transport_dec_rx_pkt

..., 1 deletion(-) > >diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c >index a1581c77cf84..2e2a773df5c1 100644 >--- a/net/vmw_vsock/virtio_transport_common.c >+++ b/net/vmw_vsock/virtio_transport_common.c >@@ -255,7 +255,7 @@ static void virtio_transport_dec_rx_pkt(struct virtio_vsock_sock *vvs, > { > int len; > >- len = skb_headroom(skb) - sizeof(struct virtio_vsock_hdr) - skb->len; >+ len = skb_headroom(skb) - sizeof(struct virtio_vsock_hdr); IIUC virtio_transport_dec_rx_pkt() is always called after skb_pull(), so skb_headroom() is retur...

...virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs, > - struct virtio_vsock_pkt *pkt) > +static void virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs, u32 len) > { > - vvs->rx_bytes += pkt->len; > + vvs->rx_bytes += len; > } > > -static void virtio_transport_dec_rx_pkt(struct virtio_vsock_sock *vvs, > - struct virtio_vsock_pkt *pkt) > +static void virtio_transport_dec_rx_pkt(struct virtio_vsock_sock *vvs, u32 len) > { > - vvs->rx_bytes -= pkt->len; > - vvs->fwd_cnt += pkt->len; > + vvs->rx_bytes -= len; > + vvs->fwd_cnt...

...2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> Maybe we could avoid this patch if we directly use pkt_len as I >>>> suggested in the previous patch. >>> Hm, may be we can avoid calling 'skb_pull()' here if 'virtio_transport_dec_rx_pkt()' >>> will use integer argument? >> >> Yep, exactly! >> >>> Just call 'virtio_transport_dec_rx_pkt(skb->len)'. skb >> >> It depends on how we call virtio_transport_inc_rx_pkt(). If we use >> hdr->len there I would use the same...

...s()->send_pkt(pkt); } -static void virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs, - struct virtio_vsock_pkt *pkt) +static void virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs, u32 len) { - vvs->rx_bytes += pkt->len; + vvs->rx_bytes += len; } -static void virtio_transport_dec_rx_pkt(struct virtio_vsock_sock *vvs, - struct virtio_vsock_pkt *pkt) +static void virtio_transport_dec_rx_pkt(struct virtio_vsock_sock *vvs, u32 len) { - vvs->rx_bytes -= pkt->len; - vvs->fwd_cnt += pkt->len; + vvs->rx_bytes -= len; + vvs->fwd_cnt += len; } void virtio_transpor...

...an 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? Thanks, Stefano

...an 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? Thanks, Stefano

While I was testing this new series (v2) I discovered an huge use of memory and a memory leak in the virtio-vsock driver in the guest when I sent 1-byte packets to the guest. These issues are present since the introduction of the virtio-vsock driver. I added the patches 1 and 2 to fix them in this series in order to better track the performance trends. v1:

While I was testing this new series (v2) I discovered an huge use of memory and a memory leak in the virtio-vsock driver in the guest when I sent 1-byte packets to the guest. These issues are present since the introduction of the virtio-vsock driver. I added the patches 1 and 2 to fix them in this series in order to better track the performance trends. v1:

...uf_alloc; /* Protected by rx_lock */ diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c index 602715fc9a75..f32301d823f5 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c @@ -206,6 +206,7 @@ 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); + vvs->last_fwd_cnt = vvs->fwd_cnt; pkt->hdr.fwd_cnt = cpu_to_le32(vvs->fwd_cnt); pkt->hdr.buf_alloc = cpu_to_le3...

...queue; > }; > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c > index 095221f94786..a85559d4d974 100644 > --- a/net/vmw_vsock/virtio_transport_common.c > +++ b/net/vmw_vsock/virtio_transport_common.c > @@ -211,6 +211,7 @@ 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); > + vvs->last_fwd_cnt = vvs->fwd_cnt; > pkt->hdr.fwd_cnt = cpu_to_le32(vvs->fwd_cnt); > pkt-...

...queue; > }; > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c > index 095221f94786..a85559d4d974 100644 > --- a/net/vmw_vsock/virtio_transport_common.c > +++ b/net/vmw_vsock/virtio_transport_common.c > @@ -211,6 +211,7 @@ 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); > + vvs->last_fwd_cnt = vvs->fwd_cnt; > pkt->hdr.fwd_cnt = cpu_to_le32(vvs->fwd_cnt); > pkt-...

...t; }; > > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c > index a85559d4d974..34a2b42313b7 100644 > --- a/net/vmw_vsock/virtio_transport_common.c > +++ b/net/vmw_vsock/virtio_transport_common.c > @@ -210,11 +210,11 @@ 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); > vvs->last_fwd_cnt = vvs->fwd_cnt; > pkt->hdr.f...

...rtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs, > - struct virtio_vsock_pkt *pkt) > +static void virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs, u32 len) > { > - vvs->rx_bytes += pkt->len; > + vvs->rx_bytes += len; > } > > -static void virtio_transport_dec_rx_pkt(struct virtio_vsock_sock *vvs, > - struct virtio_vsock_pkt *pkt) > +static void virtio_transport_dec_rx_pkt(struct virtio_vsock_sock *vvs, u32 len) > { > - vvs->rx_bytes -= pkt->len; > - vvs->fwd_cnt += pkt->len; > + vvs->rx_bytes -= len; > + vvs->fwd_cn...

...rtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs, > - struct virtio_vsock_pkt *pkt) > +static void virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs, u32 len) > { > - vvs->rx_bytes += pkt->len; > + vvs->rx_bytes += len; > } > > -static void virtio_transport_dec_rx_pkt(struct virtio_vsock_sock *vvs, > - struct virtio_vsock_pkt *pkt) > +static void virtio_transport_dec_rx_pkt(struct virtio_vsock_sock *vvs, u32 len) > { > - vvs->rx_bytes -= pkt->len; > - vvs->fwd_cnt += pkt->len; > + vvs->rx_bytes -= len; > + vvs->fwd_cn...

...s - tmp_bytes); + } else { + err = memcpy_to_msg(msg, pkt->buf + pkt->off, bytes); + if (err) + goto out; + + total += bytes; + pkt->off += bytes; + } spin_lock_bh(&vvs->rx_lock); - - total += bytes; - pkt->off += bytes; if (pkt->off == pkt->len) { virtio_transport_dec_rx_pkt(vvs, pkt); list_del(&pkt->list); @@ -1050,8 +1085,16 @@ void virtio_transport_recv_pkt(struct virtio_vsock_pkt *pkt) void virtio_transport_free_pkt(struct virtio_vsock_pkt *pkt) { - kfree(pkt->buf); - kfree(pkt); + int i; + + if (pkt->mergeable) { + for (i = 1; i < le16_to_c...

...s - tmp_bytes); + } else { + err = memcpy_to_msg(msg, pkt->buf + pkt->off, bytes); + if (err) + goto out; + + total += bytes; + pkt->off += bytes; + } spin_lock_bh(&vvs->rx_lock); - - total += bytes; - pkt->off += bytes; if (pkt->off == pkt->len) { virtio_transport_dec_rx_pkt(vvs, pkt); list_del(&pkt->list); @@ -1050,8 +1085,16 @@ void virtio_transport_recv_pkt(struct virtio_vsock_pkt *pkt) void virtio_transport_free_pkt(struct virtio_vsock_pkt *pkt) { - kfree(pkt->buf); - kfree(pkt); + int i; + + if (pkt->mergeable) { + for (i = 1; i < le16_to_c...

...t; > + */ > > > + 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 a...

...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(...

...oc; struct list_head rx_queue; }; diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c index a85559d4d974..34a2b42313b7 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c @@ -210,11 +210,11 @@ 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); vvs->last_fwd_cnt = vvs->fwd_cnt; pkt->hdr.fwd_cnt = cpu_to_le32(vvs->fwd_cn...

..._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 095221f94786..a85559d4d974 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c @@ -211,6 +211,7 @@ 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); + vvs->last_fwd_cnt = vvs->fwd_cnt; pkt->hdr.fwd_cnt = cpu_to_le32(vvs->fwd_cnt); pkt->hdr.buf_alloc = cpu_to_le3...