Mon, Jun 24, 2024 at 01:30:54PM CEST, mst@xxxxxxxxxx wrote:
>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.
What exacly do you mean by "surprise removal"?
>
>I think the cvq work also needs this, BTW.
>
>
>
>
>--
>MST
>
