Re: [PATCH V6 17/19] virtio_ring: factor out split indirect detaching logic
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: 2025-09-21 18:29:56
Also in:
lkml
On Fri, Sep 19, 2025 at 03:31:52PM +0800, Jason Wang wrote:
Factor out the split indirect descriptor detaching logic in order to make it be reused by the in order support. Acked-by: Eugenio Pérez <eperezma@redhat.com> Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> Signed-off-by: Jason Wang <jasowang@redhat.com> --- drivers/virtio/virtio_ring.c | 63 ++++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 28 deletions(-)
it is just a refactoring but the code can be prettified a bit. see below. you can make it a separate patch if you prefer but i think it is ok to do here.
quoted hunk ↗ jump to hunk
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index f376f717c8e4..5aa0cd785362 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c@@ -771,11 +771,42 @@ static bool virtqueue_kick_prepare_split(struct vring_virtqueue *vq) return needs_kick; } +static void detach_indirect_split(struct vring_virtqueue *vq, + unsigned int head) +{ + struct vring_desc_extra *extra = vq->split.desc_extra; + struct vring_desc *indir_desc = + vq->split.desc_state[head].indir_desc;
We no longer need to split this. It was like this because it was indented. By itself it fits under 80 chars even without and that is no longer a hard limit.
+ unsigned int j; + u32 len, num; + + /* Free the indirect table, if any, now that it's unmapped. */ + if (!indir_desc) + return;
en empty line won't hurt here.
+ len = vq->split.desc_extra[head].len; + + BUG_ON(!(vq->split.desc_extra[head].flags & + VRING_DESC_F_INDIRECT));
same thing here. no need to wrap anymore.
+ BUG_ON(len == 0 || len % sizeof(struct vring_desc));
+
+ num = len / sizeof(struct vring_desc);
+
+ extra = (struct vring_desc_extra *)&indir_desc[num];
+
+ if (vq->use_map_api) {
+ for (j = 0; j < num; j++)
+ vring_unmap_one_split(vq, &extra[j]);
+ }
use of {} questionable. we can keep it if you prefer though.
quoted hunk ↗ jump to hunk
+ + kfree(indir_desc); + vq->split.desc_state[head].indir_desc = NULL; +} + static void detach_buf_split(struct vring_virtqueue *vq, unsigned int head, void **ctx) { struct vring_desc_extra *extra; - unsigned int i, j; + unsigned int i; __virtio16 nextflag = cpu_to_virtio16(vq->vq.vdev, VRING_DESC_F_NEXT); /* Clear data ptr. */@@ -799,34 +830,10 @@ static void detach_buf_split(struct vring_virtqueue *vq, unsigned int head, /* Plus final descriptor */ vq->vq.num_free++; - if (vq->indirect) { - struct vring_desc *indir_desc = - vq->split.desc_state[head].indir_desc; - u32 len, num; - - /* Free the indirect table, if any, now that it's unmapped. */ - if (!indir_desc) - return; - len = vq->split.desc_extra[head].len; - - BUG_ON(!(vq->split.desc_extra[head].flags & - VRING_DESC_F_INDIRECT)); - BUG_ON(len == 0 || len % sizeof(struct vring_desc)); - - num = len / sizeof(struct vring_desc); - - extra = (struct vring_desc_extra *)&indir_desc[num]; - - if (vq->use_map_api) { - for (j = 0; j < num; j++) - vring_unmap_one_split(vq, &extra[j]); - } - - kfree(indir_desc); - vq->split.desc_state[head].indir_desc = NULL; - } else if (ctx) { + if (vq->indirect) + detach_indirect_split(vq, head); + else if (ctx) *ctx = vq->split.desc_state[head].indir_desc; - } } static bool virtqueue_poll_split(const struct vring_virtqueue *vq,-- 2.31.1