* [PATCH] virtio-vsock: Add timer to handle OOM situations @ 2016-07-18 15:11 ggarcia 2016-07-18 15:11 ` [PATCH] " ggarcia 0 siblings, 1 reply; 3+ messages in thread From: ggarcia @ 2016-07-18 15:11 UTC (permalink / raw) To: netdev; +Cc: stefanha, Gerard Garcia From: Gerard Garcia <ggarcia@deic.uab.cat> This patch applies over the stefanha/vsock-next repository: https://github.com/stefanha/linux/tree/vsock-next Once the guest puts a packet in the virtqueue it is guaranteed that it will be delivered. This patch addresses the problem of packets being discarded when there is not available memory in the host. Gerard Garcia (1): Add timer to handle OOM situations drivers/vhost/vsock.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 47 insertions(+), 5 deletions(-) -- 2.9.1 ^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] Add timer to handle OOM situations 2016-07-18 15:11 [PATCH] virtio-vsock: Add timer to handle OOM situations ggarcia @ 2016-07-18 15:11 ` ggarcia 2016-07-20 9:36 ` Stefan Hajnoczi 0 siblings, 1 reply; 3+ messages in thread From: ggarcia @ 2016-07-18 15:11 UTC (permalink / raw) To: netdev; +Cc: stefanha, Gerard Garcia From: Gerard Garcia <ggarcia@deic.uab.cat> Better testing support files Signed-off-by: Gerard Garcia <ggarcia@deic.uab.cat> --- drivers/vhost/vsock.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 47 insertions(+), 5 deletions(-) diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c index 17bfe4e..d746454 100644 --- a/drivers/vhost/vsock.c +++ b/drivers/vhost/vsock.c @@ -14,11 +14,13 @@ #include <net/sock.h> #include <linux/virtio_vsock.h> #include <linux/vhost.h> +#include <linux/timer.h> #include <net/af_vsock.h> #include "vhost.h" #define VHOST_VSOCK_DEFAULT_HOST_CID 2 +#define OOM_RETRY_MS 100 enum { VHOST_VSOCK_FEATURES = VHOST_FEATURES, @@ -43,8 +45,12 @@ struct vhost_vsock { u32 total_tx_buf; u32 guest_cid; + + struct timer_list tx_kick; }; + + static u32 vhost_transport_get_local_cid(void) { return VHOST_VSOCK_DEFAULT_HOST_CID; @@ -273,7 +279,8 @@ vhost_transport_send_pkt(struct vsock_sock *vsk, static struct virtio_vsock_pkt * vhost_vsock_alloc_pkt(struct vhost_virtqueue *vq, - unsigned int out, unsigned int in) + unsigned int out, unsigned int in, + int *error) { struct virtio_vsock_pkt *pkt; struct iov_iter iov_iter; @@ -282,12 +289,15 @@ vhost_vsock_alloc_pkt(struct vhost_virtqueue *vq, if (in != 0) { vq_err(vq, "Expected 0 input buffers, got %u\n", in); + *error = -EINVAL; return NULL; } pkt = kzalloc(sizeof(*pkt), GFP_KERNEL); - if (!pkt) + if (!pkt){ + *error = -ENOMEM; return NULL; + } len = iov_length(vq->iov, out); iov_iter_init(&iov_iter, WRITE, vq->iov, out, len); @@ -297,6 +307,7 @@ vhost_vsock_alloc_pkt(struct vhost_virtqueue *vq, vq_err(vq, "Expected %zu bytes for pkt->hdr, got %zu bytes\n", sizeof(pkt->hdr), nbytes); kfree(pkt); + *error = -EINVAL; return NULL; } @@ -310,12 +321,14 @@ vhost_vsock_alloc_pkt(struct vhost_virtqueue *vq, /* The pkt is too big */ if (pkt->len > VIRTIO_VSOCK_MAX_PKT_BUF_SIZE) { kfree(pkt); + *error = -EINVAL; return NULL; } pkt->buf = kmalloc(pkt->len, GFP_KERNEL); if (!pkt->buf) { kfree(pkt); + *error = -ENOMEM; return NULL; } @@ -324,6 +337,7 @@ vhost_vsock_alloc_pkt(struct vhost_virtqueue *vq, vq_err(vq, "Expected %u byte payload, got %zu bytes\n", pkt->len, nbytes); virtio_transport_free_pkt(pkt); + *error = -EINVAL; return NULL; } @@ -340,6 +354,7 @@ static void vhost_vsock_handle_tx_kick(struct vhost_work *work) int head; unsigned int out, in; bool added = false; + int error; mutex_lock(&vq->mutex); @@ -361,10 +376,27 @@ static void vhost_vsock_handle_tx_kick(struct vhost_work *work) break; } - pkt = vhost_vsock_alloc_pkt(vq, out, in); + pkt = vhost_vsock_alloc_pkt(vq, out, in, &error); + if (!pkt) { - vq_err(vq, "Faulted on pkt\n"); - continue; + if (error == -ENOMEM) { + vhost_discard_vq_desc(vq, 1); + + if (!timer_pending(&vsock->tx_kick)) { + vsock->tx_kick.data = + (unsigned long) vq; + vsock->tx_kick.expires = + jiffies + msecs_to_jiffies(OOM_RETRY_MS); + add_timer(&vsock->tx_kick); + } + + break; + } else { + vq_err(vq, "Faulted on pkt\n"); + continue; + } + } else if (unlikely(timer_pending(&vsock->tx_kick))) { + del_timer(&vsock->tx_kick); } /* Only accept correctly addressed packets */ @@ -383,6 +415,13 @@ out: mutex_unlock(&vq->mutex); } +static void vhost_vsock_rehandle_tx_kick(unsigned long data) +{ + struct vhost_virtqueue *vq = (struct vhost_virtqueue *) data; + + vhost_poll_queue(&vq->poll); +} + static void vhost_vsock_handle_rx_kick(struct vhost_work *work) { struct vhost_virtqueue *vq = container_of(work, struct vhost_virtqueue, @@ -493,6 +532,9 @@ static int vhost_vsock_dev_open(struct inode *inode, struct file *file) goto out; } + setup_timer(&vsock->tx_kick, + vhost_vsock_rehandle_tx_kick, (unsigned long) NULL); + vqs[VSOCK_VQ_TX] = &vsock->vqs[VSOCK_VQ_TX]; vqs[VSOCK_VQ_RX] = &vsock->vqs[VSOCK_VQ_RX]; vsock->vqs[VSOCK_VQ_TX].handle_kick = vhost_vsock_handle_tx_kick; -- 2.9.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] Add timer to handle OOM situations 2016-07-18 15:11 ` [PATCH] " ggarcia @ 2016-07-20 9:36 ` Stefan Hajnoczi 0 siblings, 0 replies; 3+ messages in thread From: Stefan Hajnoczi @ 2016-07-20 9:36 UTC (permalink / raw) To: ggarcia; +Cc: netdev [-- Attachment #1: Type: text/plain, Size: 4803 bytes --] On Mon, Jul 18, 2016 at 05:11:03PM +0200, ggarcia@abra.uab.cat wrote: > From: Gerard Garcia <ggarcia@deic.uab.cat> > > Better testing support files > > Signed-off-by: Gerard Garcia <ggarcia@deic.uab.cat> > --- > drivers/vhost/vsock.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 47 insertions(+), 5 deletions(-) > > diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c > index 17bfe4e..d746454 100644 > --- a/drivers/vhost/vsock.c > +++ b/drivers/vhost/vsock.c > @@ -14,11 +14,13 @@ > #include <net/sock.h> > #include <linux/virtio_vsock.h> > #include <linux/vhost.h> > +#include <linux/timer.h> > > #include <net/af_vsock.h> > #include "vhost.h" > > #define VHOST_VSOCK_DEFAULT_HOST_CID 2 > +#define OOM_RETRY_MS 100 > > enum { > VHOST_VSOCK_FEATURES = VHOST_FEATURES, > @@ -43,8 +45,12 @@ struct vhost_vsock { > u32 total_tx_buf; > > u32 guest_cid; > + > + struct timer_list tx_kick; > }; > > + > + > static u32 vhost_transport_get_local_cid(void) > { > return VHOST_VSOCK_DEFAULT_HOST_CID; > @@ -273,7 +279,8 @@ vhost_transport_send_pkt(struct vsock_sock *vsk, > > static struct virtio_vsock_pkt * > vhost_vsock_alloc_pkt(struct vhost_virtqueue *vq, > - unsigned int out, unsigned int in) > + unsigned int out, unsigned int in, > + int *error) You can use <linux/err.h> ERR_PTR(errno) and IS_ERR(ptr)/PTR_ERR() instead of adding int *error. > { > struct virtio_vsock_pkt *pkt; > struct iov_iter iov_iter; > @@ -282,12 +289,15 @@ vhost_vsock_alloc_pkt(struct vhost_virtqueue *vq, > > if (in != 0) { > vq_err(vq, "Expected 0 input buffers, got %u\n", in); > + *error = -EINVAL; > return NULL; > } > > pkt = kzalloc(sizeof(*pkt), GFP_KERNEL); > - if (!pkt) > + if (!pkt){ > + *error = -ENOMEM; > return NULL; > + } > > len = iov_length(vq->iov, out); > iov_iter_init(&iov_iter, WRITE, vq->iov, out, len); > @@ -297,6 +307,7 @@ vhost_vsock_alloc_pkt(struct vhost_virtqueue *vq, > vq_err(vq, "Expected %zu bytes for pkt->hdr, got %zu bytes\n", > sizeof(pkt->hdr), nbytes); > kfree(pkt); > + *error = -EINVAL; > return NULL; > } > > @@ -310,12 +321,14 @@ vhost_vsock_alloc_pkt(struct vhost_virtqueue *vq, > /* The pkt is too big */ > if (pkt->len > VIRTIO_VSOCK_MAX_PKT_BUF_SIZE) { > kfree(pkt); > + *error = -EINVAL; > return NULL; > } > > pkt->buf = kmalloc(pkt->len, GFP_KERNEL); > if (!pkt->buf) { > kfree(pkt); > + *error = -ENOMEM; > return NULL; > } > > @@ -324,6 +337,7 @@ vhost_vsock_alloc_pkt(struct vhost_virtqueue *vq, > vq_err(vq, "Expected %u byte payload, got %zu bytes\n", > pkt->len, nbytes); > virtio_transport_free_pkt(pkt); > + *error = -EINVAL; > return NULL; > } > > @@ -340,6 +354,7 @@ static void vhost_vsock_handle_tx_kick(struct vhost_work *work) > int head; > unsigned int out, in; > bool added = false; > + int error; > > mutex_lock(&vq->mutex); > > @@ -361,10 +376,27 @@ static void vhost_vsock_handle_tx_kick(struct vhost_work *work) > break; > } > > - pkt = vhost_vsock_alloc_pkt(vq, out, in); > + pkt = vhost_vsock_alloc_pkt(vq, out, in, &error); > + > if (!pkt) { > - vq_err(vq, "Faulted on pkt\n"); > - continue; > + if (error == -ENOMEM) { > + vhost_discard_vq_desc(vq, 1); > + > + if (!timer_pending(&vsock->tx_kick)) { > + vsock->tx_kick.data = > + (unsigned long) vq; > + vsock->tx_kick.expires = > + jiffies + msecs_to_jiffies(OOM_RETRY_MS); > + add_timer(&vsock->tx_kick); > + } > + > + break; > + } else { > + vq_err(vq, "Faulted on pkt\n"); > + continue; Please change continue to break (like drivers/vhost/net.c). Let's not process packets after an invalid one. > + } > + } else if (unlikely(timer_pending(&vsock->tx_kick))) { > + del_timer(&vsock->tx_kick); > } > > /* Only accept correctly addressed packets */ > @@ -383,6 +415,13 @@ out: > mutex_unlock(&vq->mutex); > } > > +static void vhost_vsock_rehandle_tx_kick(unsigned long data) > +{ > + struct vhost_virtqueue *vq = (struct vhost_virtqueue *) data; > + > + vhost_poll_queue(&vq->poll); > +} > + > static void vhost_vsock_handle_rx_kick(struct vhost_work *work) > { > struct vhost_virtqueue *vq = container_of(work, struct vhost_virtqueue, > @@ -493,6 +532,9 @@ static int vhost_vsock_dev_open(struct inode *inode, struct file *file) > goto out; > } > > + setup_timer(&vsock->tx_kick, > + vhost_vsock_rehandle_tx_kick, (unsigned long) NULL); > + Where is the timer deleted when the vhost_vsock instance is released? [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-07-20 11:32 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-07-18 15:11 [PATCH] virtio-vsock: Add timer to handle OOM situations ggarcia 2016-07-18 15:11 ` [PATCH] " ggarcia 2016-07-20 9:36 ` Stefan Hajnoczi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox