Xuan Zhuo
2022-Mar-08 12:35 UTC
[PATCH v7 08/26] virtio_ring: extract the logic of freeing vring
Introduce vring_free() to free the vring of vq.
Prevent double free by setting vq->reset.
Signed-off-by: Xuan Zhuo <xuanzhuo at linux.alibaba.com>
---
drivers/virtio/virtio_ring.c | 25 ++++++++++++++++++++-----
include/linux/virtio.h | 8 ++++++++
2 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index b5a9bf4f45b3..e0422c04c903 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -2442,14 +2442,10 @@ struct virtqueue *vring_new_virtqueue(unsigned int
index,
}
EXPORT_SYMBOL_GPL(vring_new_virtqueue);
-void vring_del_virtqueue(struct virtqueue *_vq)
+static void __vring_free(struct virtqueue *_vq)
{
struct vring_virtqueue *vq = to_vvq(_vq);
- spin_lock(&vq->vq.vdev->vqs_list_lock);
- list_del(&_vq->list);
- spin_unlock(&vq->vq.vdev->vqs_list_lock);
-
if (vq->we_own_ring) {
if (vq->packed_ring) {
vring_free_queue(vq->vq.vdev,
@@ -2480,6 +2476,25 @@ void vring_del_virtqueue(struct virtqueue *_vq)
kfree(vq->split.desc_state);
kfree(vq->split.desc_extra);
}
+}
+
+static void vring_free(struct virtqueue *vq)
+{
+ __vring_free(vq);
+ vq->reset = VIRTIO_VQ_RESET_STEP_VRING_RELEASE;
+}
+
+void vring_del_virtqueue(struct virtqueue *_vq)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+
+ spin_lock(&vq->vq.vdev->vqs_list_lock);
+ list_del(&_vq->list);
+ spin_unlock(&vq->vq.vdev->vqs_list_lock);
+
+ if (_vq->reset != VIRTIO_VQ_RESET_STEP_VRING_RELEASE)
+ __vring_free(_vq);
+
kfree(vq);
}
EXPORT_SYMBOL_GPL(vring_del_virtqueue);
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index d59adc4be068..e3714e6db330 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -10,6 +10,13 @@
#include <linux/mod_devicetable.h>
#include <linux/gfp.h>
+enum virtio_vq_reset_step {
+ VIRTIO_VQ_RESET_STEP_NONE,
+ VIRTIO_VQ_RESET_STEP_DEVICE,
+ VIRTIO_VQ_RESET_STEP_VRING_RELEASE,
+ VIRTIO_VQ_RESET_STEP_VRING_ATTACH,
+};
+
/**
* virtqueue - a queue to register buffers for sending or receiving.
* @list: the chain of virtqueues for this device
@@ -33,6 +40,7 @@ struct virtqueue {
unsigned int num_free;
unsigned int num_max;
void *priv;
+ enum virtio_vq_reset_step reset;
};
int virtqueue_add_outbuf(struct virtqueue *vq,
--
2.31.0
Jason Wang
2022-Mar-09 07:51 UTC
[PATCH v7 08/26] virtio_ring: extract the logic of freeing vring
? 2022/3/8 ??8:35, Xuan Zhuo ??:> Introduce vring_free() to free the vring of vq. > > Prevent double free by setting vq->reset. > > Signed-off-by: Xuan Zhuo <xuanzhuo at linux.alibaba.com> > --- > drivers/virtio/virtio_ring.c | 25 ++++++++++++++++++++----- > include/linux/virtio.h | 8 ++++++++ > 2 files changed, 28 insertions(+), 5 deletions(-) > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c > index b5a9bf4f45b3..e0422c04c903 100644 > --- a/drivers/virtio/virtio_ring.c > +++ b/drivers/virtio/virtio_ring.c > @@ -2442,14 +2442,10 @@ struct virtqueue *vring_new_virtqueue(unsigned int index, > } > EXPORT_SYMBOL_GPL(vring_new_virtqueue); > > -void vring_del_virtqueue(struct virtqueue *_vq) > +static void __vring_free(struct virtqueue *_vq) > { > struct vring_virtqueue *vq = to_vvq(_vq); > > - spin_lock(&vq->vq.vdev->vqs_list_lock); > - list_del(&_vq->list); > - spin_unlock(&vq->vq.vdev->vqs_list_lock); > - > if (vq->we_own_ring) { > if (vq->packed_ring) { > vring_free_queue(vq->vq.vdev, > @@ -2480,6 +2476,25 @@ void vring_del_virtqueue(struct virtqueue *_vq) > kfree(vq->split.desc_state); > kfree(vq->split.desc_extra); > } > +} > + > +static void vring_free(struct virtqueue *vq) > +{ > + __vring_free(vq); > + vq->reset = VIRTIO_VQ_RESET_STEP_VRING_RELEASE; > +} > + > +void vring_del_virtqueue(struct virtqueue *_vq) > +{ > + struct vring_virtqueue *vq = to_vvq(_vq); > + > + spin_lock(&vq->vq.vdev->vqs_list_lock); > + list_del(&_vq->list); > + spin_unlock(&vq->vq.vdev->vqs_list_lock); > + > + if (_vq->reset != VIRTIO_VQ_RESET_STEP_VRING_RELEASE) > + __vring_free(_vq); > + > kfree(vq); > } > EXPORT_SYMBOL_GPL(vring_del_virtqueue); > diff --git a/include/linux/virtio.h b/include/linux/virtio.h > index d59adc4be068..e3714e6db330 100644 > --- a/include/linux/virtio.h > +++ b/include/linux/virtio.h > @@ -10,6 +10,13 @@ > #include <linux/mod_devicetable.h> > #include <linux/gfp.h> > > +enum virtio_vq_reset_step { > + VIRTIO_VQ_RESET_STEP_NONE, > + VIRTIO_VQ_RESET_STEP_DEVICE, > + VIRTIO_VQ_RESET_STEP_VRING_RELEASE, > + VIRTIO_VQ_RESET_STEP_VRING_ATTACH, > +};This part looks not related to the subject. And it needs detail documentation on this. But I wonder how useful it is, anyway we can check the reset status via transport specific way and in the future we may want to do more than just resizing (e.g PASID). Thanks> + > /** > * virtqueue - a queue to register buffers for sending or receiving. > * @list: the chain of virtqueues for this device > @@ -33,6 +40,7 @@ struct virtqueue { > unsigned int num_free; > unsigned int num_max; > void *priv; > + enum virtio_vq_reset_step reset; > }; > > int virtqueue_add_outbuf(struct virtqueue *vq,