search for: virtqueue_detach_unused_buf

...ndex fbeb71953526..5fbf2ac73111 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -1506,15 +1506,25 @@ static void remove_port(struct kref *kref) kfree(port); } +static void remove_unused_bufs(struct virtqueue *vq) +{ + struct port_buffer *buf; + + while ((buf = virtqueue_detach_unused_buf(vq))) + free_buf(buf, true); +} + static void remove_port_data(struct port *port) { spin_lock_irq(&port->inbuf_lock); /* Remove unused data this port might have received. */ discard_port_data(port); + remove_unused_bufs(port->in_vq); spin_unlock_irq(&port->inbuf_lock);...

...ndex fbeb71953526..5fbf2ac73111 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -1506,15 +1506,25 @@ static void remove_port(struct kref *kref) kfree(port); } +static void remove_unused_bufs(struct virtqueue *vq) +{ + struct port_buffer *buf; + + while ((buf = virtqueue_detach_unused_buf(vq))) + free_buf(buf, true); +} + static void remove_port_data(struct port *port) { spin_lock_irq(&port->inbuf_lock); /* Remove unused data this port might have received. */ discard_port_data(port); + remove_unused_bufs(port->in_vq); spin_unlock_irq(&port->inbuf_lock);...

...t.c > index 8d8038538fc4..a12ae26db0e2 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -3560,12 +3560,14 @@ static void free_unused_bufs(struct virtnet_info *vi) > struct virtqueue *vq = vi->sq[i].vq; > while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) > virtnet_sq_free_unused_buf(vq, buf); > + cond_resched(); Does this really address the case when the virtqueue is very large? Thanks > } > > for (i = 0; i < vi->max_queue_pairs; i++) { >...

This patch makes packed ring code compatible with split ring in function 'virtqueue_detach_unused_buf_*'. Signed-off-by: Pankaj Gupta <pagupta at redhat.com> --- drivers/virtio/virtio_ring.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index c8be1c4f5b55..1b98a6777b7e 100644 --- a/drivers/virtio/virtio_ring.c +++ b/...

On Thu, Aug 08, 2019 at 05:06:06PM +0530, Pankaj Gupta wrote: > This patch makes packed ring code compatible with split ring in function > 'virtqueue_detach_unused_buf_*'. What does that mean? What does this "fix"? thanks, greg k-h

> > This patch makes packed ring code compatible with split ring in function > > 'virtqueue_detach_unused_buf_*'. > > What does that mean? What does this "fix"? Patch 1 frees the buffers When a port is unplugged from the virtio console device. It does this with the help of 'virtqueue_detach_unused_buf_split/packed' function. For split ring case, corresponding function decreme...

On Thu, Aug 08, 2019 at 08:28:46AM -0400, Pankaj Gupta wrote: > > > > > This patch makes packed ring code compatible with split ring in function > > > 'virtqueue_detach_unused_buf_*'. > > > > What does that mean? What does this "fix"? > > Patch 1 frees the buffers When a port is unplugged from the virtio > console device. It does this with the help of 'virtqueue_detach_unused_buf_split/packed' > function. For split ring case,...

On Thu, Aug 08, 2019 at 05:06:06PM +0530, Pankaj Gupta wrote: > This patch makes packed ring code compatible with split ring in function > 'virtqueue_detach_unused_buf_*'. What does that mean? What does this "fix"? thanks, greg k-h

...> > @@ -1506,15 +1506,25 @@ static void remove_port(struct kref *kref) > > kfree(port); > > } > > > > +static void remove_unused_bufs(struct virtqueue *vq) > > +{ > > + struct port_buffer *buf; > > + > > + while ((buf = virtqueue_detach_unused_buf(vq))) > > + free_buf(buf, true); > > +} > > + > > static void remove_port_data(struct port *port) > > { > > spin_lock_irq(&port->inbuf_lock); > > /* Remove unused data this port might have received. */ > >...

...> > @@ -1506,15 +1506,25 @@ static void remove_port(struct kref *kref) > > kfree(port); > > } > > > > +static void remove_unused_bufs(struct virtqueue *vq) > > +{ > > + struct port_buffer *buf; > > + > > + while ((buf = virtqueue_detach_unused_buf(vq))) > > + free_buf(buf, true); > > +} > > + > > static void remove_port_data(struct port *port) > > { > > spin_lock_irq(&port->inbuf_lock); > > /* Remove unused data this port might have received. */ > >...

...> + vring_unmap_one_packed(vq, desc); > > > > > > > > + desc->flags = 0x0; > > > > > > > Looks like this is unnecessary. > > > > > > It's safer to zero it. If we don't zero it, after we > > > > > > call virtqueue_detach_unused_buf_packed() which calls > > > > > > this function, the desc is still available to the > > > > > > device. > > > > > > > > > > Well detach_unused_buf_packed() should be called after device is stopped, > > > > > otherwise...

...> + vring_unmap_one_packed(vq, desc); > > > > > > > > + desc->flags = 0x0; > > > > > > > Looks like this is unnecessary. > > > > > > It's safer to zero it. If we don't zero it, after we > > > > > > call virtqueue_detach_unused_buf_packed() which calls > > > > > > this function, the desc is still available to the > > > > > > device. > > > > > > > > > > Well detach_unused_buf_packed() should be called after device is stopped, > > > > > otherwise...

...est 3. hot-unplug while the device is busy serving I/O Test: ~1000 rounds of hot-plug/hot-unplug test passed with this patch. Changes in v3: - Drop blk_abort_queue and blk_abort_request - Use __blk_end_request_all to complete request dispatched to driver Changes in v2: - Drop req_in_flight - Use virtqueue_detach_unused_buf to get request dispatched to driver Signed-off-by: Asias He <asias at redhat.com> --- drivers/block/virtio_blk.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c index 72fe55d..693187d 100644 --- a/drivers/block/virti...

...est 3. hot-unplug while the device is busy serving I/O Test: ~1000 rounds of hot-plug/hot-unplug test passed with this patch. Changes in v3: - Drop blk_abort_queue and blk_abort_request - Use __blk_end_request_all to complete request dispatched to driver Changes in v2: - Drop req_in_flight - Use virtqueue_detach_unused_buf to get request dispatched to driver Signed-off-by: Asias He <asias at redhat.com> --- drivers/block/virtio_blk.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c index 72fe55d..693187d 100644 --- a/drivers/block/virti...

...c4..a12ae26db0e2 100644 > > --- a/drivers/net/virtio_net.c > > +++ b/drivers/net/virtio_net.c > > @@ -3560,12 +3560,14 @@ static void free_unused_bufs(struct virtnet_info *vi) > > struct virtqueue *vq = vi->sq[i].vq; > > while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) > > virtnet_sq_free_unused_buf(vq, buf); > > + cond_resched(); > > Does this really address the case when the virtqueue is very large? > > Thanks it does in that a very large queue is still just 64k in size. we might ho...

...d_port(struct ports_device *portdev, u32 id) { char debugfs_name[16]; struct port *port; - struct port_buffer *buf; dev_t devt; unsigned int nr_added_bufs; int err; @@ -1513,8 +1512,6 @@ static int add_port(struct ports_device *portdev, u32 id) return 0; free_inbufs: - while ((buf = virtqueue_detach_unused_buf(port->in_vq))) - free_buf(buf, true); free_device: device_destroy(pdrvdata.class, port->dev->devt); free_cdev: @@ -1539,34 +1536,14 @@ static void remove_port(struct kref *kref) static void remove_port_data(struct port *port) { - struct port_buffer *buf; - spin_lock_irq(&por...

...rs/char/virtio_console.c > +++ b/drivers/char/virtio_console.c > @@ -1494,15 +1494,25 @@ static void remove_port(struct kref *kref) > kfree(port); > } > > +static void remove_unused_bufs(struct virtqueue *vq) > +{ > + struct port_buffer *buf; > + > + while ((buf = virtqueue_detach_unused_buf(vq))) > + free_buf(buf, true); > +} > + > static void remove_port_data(struct port *port) > { > spin_lock_irq(&port->inbuf_lock); > /* Remove unused data this port might have received. */ > discard_port_data(port); > + remove_unused_bufs(port->in_vq); &...

...rs/char/virtio_console.c > +++ b/drivers/char/virtio_console.c > @@ -1494,15 +1494,25 @@ static void remove_port(struct kref *kref) > kfree(port); > } > > +static void remove_unused_bufs(struct virtqueue *vq) > +{ > + struct port_buffer *buf; > + > + while ((buf = virtqueue_detach_unused_buf(vq))) > + free_buf(buf, true); > +} > + > static void remove_port_data(struct port *port) > { > spin_lock_irq(&port->inbuf_lock); > /* Remove unused data this port might have received. */ > discard_port_data(port); > + remove_unused_bufs(port->in_vq); &...

...the device is already stopped. We'll have q->in_flight[] > 0, so the drain will not finish. How to reproduce the race: 1. hot-plug a virtio-blk device 2. keep reading/writing the device in guest 3. hot-unplug while the device is busy serving I/O Changes in v2: - Drop req_in_flight - Use virtqueue_detach_unused_buf to get request dispatched to driver Signed-off-by: Asias He <asias at redhat.com> --- drivers/block/virtio_blk.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c index 72fe55d..670c28f 100644 ---...

...the device is already stopped. We'll have q->in_flight[] > 0, so the drain will not finish. How to reproduce the race: 1. hot-plug a virtio-blk device 2. keep reading/writing the device in guest 3. hot-unplug while the device is busy serving I/O Changes in v2: - Drop req_in_flight - Use virtqueue_detach_unused_buf to get request dispatched to driver Signed-off-by: Asias He <asias at redhat.com> --- drivers/block/virtio_blk.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c index 72fe55d..670c28f 100644 ---...