Re: [Qemu-devel] [PATCH] eventfd: making it thread safe

[Alexey Kardashevskiy <aik@ozlabs.ru>]

On 18/07/12 22:22, Michael S. Tsirkin wrote:
> On Wed, Jul 18, 2012 at 10:08:53PM +1000, Alexey Kardashevskiy wrote:
>> QEMU uses IO handlers to run select() in the main loop.
>> The handlers list is managed by qemu_set_fd_handler() helper
>> which works fine when called from the main thread as it is
>> called not when select() is waiting.
> 
> when select() is not waiting?
> 
>>
>> However IO handlers list can be changed in the thread other than
>> the main one doing os_host_main_loop_wait(), for example, as a result
>> of a hypercall which changes PCI config space (VFIO on POWER is the case)
> 
> So the problem is only with VFIO? Can it affect vhost-net?

Honestly I have no idea about vhost-net as I never tried it.


>> and enables/disabled MSI/MSIX which closes/creates eventfd handles.
> 
> There doesn't seem to be a notification in case an fd is
> deleted. It's probably not at all urgent to remove
> an fd from select - why do you mention closing handles?

Agrhh. I missed this comment in the patch I just reposted.
Mentioned because the file* is still open when there is no need in it.
It has no effect for eventfd but may have for somebody else so we probably
want to add a notification on deletion. Dunno.


>> If the main loop is waiting on such eventfd, it has to be restarted.
> 
> Do you really mean 'should be waiting on the newly created
> eventfd'?
> 
>> The patch adds the qemu_notify_event() call to interrupt select()
>> and make main_loop() to restart select()
> 
> s/and make main_loop() to restart/to make main_loop() restart/?

Thanks for the comments. David used to polish my english but he is vacation
now :)


>> with the updated IO
>> handlers list.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  iohandler.c |    1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/iohandler.c b/iohandler.c
>> index 3c74de6..dea4355 100644
>> --- a/iohandler.c
>> +++ b/iohandler.c
>> @@ -77,6 +77,7 @@ int qemu_set_fd_handler2(int fd,
>>          ioh->fd_write = fd_write;
>>          ioh->opaque = opaque;
>>          ioh->deleted = 0;
>> +        qemu_notify_event();
>>      }
>>      return 0;
>>  }
>> -- 
>> 1.7.10.4


-- 
Alexey