On Mon, Jun 24, 2024 at 11:04:50AM +0200, Jiri Pirko wrote:
> @@ -73,21 +91,30 @@ static int virtqueue_exec_admin_cmd(struct
> virtio_pci_device *vp_dev,
> !((1ULL << opcode) & admin_vq->supported_cmds))
> return -EOPNOTSUPP;
>
> - ret = virtqueue_add_sgs(vq, sgs, out_num, in_num, cmd, GFP_KERNEL);
> - if (ret < 0)
> - return -EIO;
> -
> - if (unlikely(!virtqueue_kick(vq)))
> - return -EIO;
> + init_completion(&cmd->completion);
>
> - while (!virtqueue_get_buf(vq, &len) &&
> - !virtqueue_is_broken(vq))
> - cpu_relax();
> +again:
> + spin_lock_irqsave(&admin_vq->lock, flags);
> + ret = virtqueue_add_sgs(vq, sgs, out_num, in_num, cmd, GFP_KERNEL);
> + if (ret < 0) {
> + if (ret == -ENOSPC) {
> + spin_unlock_irqrestore(&admin_vq->lock, flags);
> + cpu_relax();
> + goto again;
> + }
> + goto unlock_err;
> + }
> + if (WARN_ON_ONCE(!virtqueue_kick(vq)))
> + goto unlock_err;
> + spin_unlock_irqrestore(&admin_vq->lock, flags);
>
> - if (virtqueue_is_broken(vq))
> - return -EIO;
> + wait_for_completion(&cmd->completion);
>
> return 0;
> +
> +unlock_err:
> + spin_unlock_irqrestore(&admin_vq->lock, flags);
> + return -EIO;
> }
>
The reason we had the virtqueue_is_broken check previously,
is because this way surprise removal works: it happens to set
vq broken flag on all vqs.
So if you are changing that to a completion, I think surprise
removal needs to trigger a callback so the completion
can be signalled.
I think the cvq work also needs this, BTW.
--
MST
