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 <[email protected]>
> Reviewed-by: Xuan Zhuo <[email protected]>
> Signed-off-by: Jason Wang <[email protected]>
> ---
> 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.
> 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.
> +
> + 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
