Re: [PATCH v2 2/2] vdpa: commit all host notifier MRs in a single MR transaction

[longpeng2--- via <qemu-devel@nongnu.org>]

在 2022/12/6 16:30, Philippe Mathieu-Daudé 写道:
> On 6/12/22 09:18, Longpeng(Mike) via wrote:
>> From: Longpeng <longpeng2@huawei.com>
>>
>> This allows the vhost-vdpa device to batch the setup of all its MRs of
>> host notifiers.
>>
>> This significantly reduces the device starting time, e.g. the time spend
>> on setup the host notifier MRs reduce from 423ms to 32ms for a VM with
>> 64 vCPUs and 3 vhost-vDPA generic devices[1] (64vq per device).
>>
>> [1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg921541.html
>>
>> Signed-off-by: Longpeng <longpeng2@huawei.com>
>> ---
>>   hw/virtio/vhost-vdpa.c | 18 ++++++++++++++++++
>>   1 file changed, 18 insertions(+)
> 
> 
>> @@ -562,16 +571,25 @@ static void 
>> vhost_vdpa_host_notifiers_init(struct vhost_dev *dev)
>>           return;
>>       }
>> +    /*
>> +     * Pack all the changes to the memory regions in a single
>> +     * transaction to avoid a few updating of the address space
>> +     * topology.
>> +     */
>> +    memory_region_transaction_begin();
>> +
>>       for (i = dev->vq_index; i < dev->vq_index + dev->nvqs; i++) {
>>           if (vhost_vdpa_host_notifier_init(dev, i)) {
>>               goto err;
> 
> Could we simplify by replacing this goto statement with:
> 
>                 vhost_vdpa_host_notifiers_uninit(dev, i - dev->vq_index);
>                 break;
> 
> ?
> 
Good suggestion! I'll do in v3, thanks.

>>           }
>>       }
>> +    memory_region_transaction_commit();
>>       return;
>>   err:
>>       vhost_vdpa_host_notifiers_uninit(dev, i - dev->vq_index);
>> +    memory_region_transaction_commit();
>>       return;
>>   }
> 
> .