Mon, Jun 24, 2024 at 01:30:54PM CEST, m...@redhat.com 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.

Got it. Will check that out.


>
>I think the cvq work also needs this, BTW.
>
>
>
>
>-- 
>MST
>

Reply via email to