>> >Maybe this is a race case only in my scenario, I need to dig further.
>> >Thanks for your analysis.
>>
>> Sorry for the noise, find root cause, this is indeed a race case only in my
>> scenario, because I need to delete fault_fd from polling fds in vcpu thread.
>>
>> While aio_set_fd_handler() indeed wake up the main thread from poll(),
>> but there still could be a small window we call free_id(fault_id) before
>> poll() exit and release fault_id file reference.
>>
>> I wrote below to make it work, in worst case I see free_id(fault_id) failed
>> once
>> and could succeed in second try.
>>
>> --- a/hw/i386/intel_iommu_accel.c
>> +++ b/hw/i386/intel_iommu_accel.c
>> @@ -206,13 +206,20 @@ static void
>> vtd_destroy_fs_faultq(VTDHostIOMMUDevice *vtd_hiod,
>> uint32_t fault_id, uint32_t fault_fd)
>> {
>> HostIOMMUDeviceIOMMUFD *idev =
>> HOST_IOMMU_DEVICE_IOMMUFD(vtd_hiod->hiod);
>> + int ret, try_cnt = 5;
>>
>> if (!fault_id) {
>> return;
>> }
>>
>> close(fault_fd);
>> - iommufd_backend_free_id(idev->iommufd, fault_id);
>> +
>> + while(try_cnt--) {
>> + ret = iommufd_backend_free_id(idev->iommufd, fault_id);
>> + if (!ret || errno != EBUSY ) {
>> + break;
>> + }
>> + }
>> }
>
>Is it possible for you to use aio_bh_schedule_oneshot() here to force the
>unregister run in QEMU I/O handler AioContext instead. I think that is
>what this fn does( though I have not used this before).
>
>Something like,
>
>aio_bh_schedule_oneshot(iohandler_get_aio_context(),
> faultq_teardown_bh,
> data);
>
>and
>
>static void faultq_teardown_bh(void *opaque)
>{
> data *fq = opaque;
>
> qemu_set_fd_handler(fq->fd, NULL, NULL, NULL);
>
> close(fq->fd);
>
> iommufd_backend_free_id(fq->iommufd, fq->id);
>}
Good suggestion, I just confirmed this works fine for my scenario.
Thanks
Zhenzhong
