On 7/1/20 6:03 PM, Stefan Hajnoczi wrote:
> On Tue, Jun 30, 2020 at 09:13:18PM +0200, Philippe Mathieu-Daudé wrote:
>> To be able to use multiple queues on the same hardware,
>> we need to have each queue able to receive IRQ notifications
>> in the correct AIO context.
>> The AIO context and the notification handler have to be proper
>> to each queue, not to the block driver. Move aio_context and
>> irq_notifier from BDRVNVMeState to NVMeQueuePair.
>
> If I understand correctly, after this patch:
>
> 1. Each queue pair has an EventNotifier irq_notifier but only the admin
>queuepair's irq_notifier is hooked up to VFIO. This means all
>interrupts come via the admin queuepair.
Yes, but this was the behavior before this patch too.
>
>(This also means all queuepairs still need to be polled for
>completions when the interrupt fires because we don't know which
>queuepair had a completion event.)
Yes.
>
> 2. AioContexts must be identical across all queuepairs and
>BlockDriverStates. Although they all have their own AioContext
>pointer there is no true support for different AioContexts yet.
I'm not sure. Maybe v3 will sort that out.
>
>(For example, nvme_cmd_sync() is called with a bs argument but
>AIO_WAIT_WHILE(q->aio_context, ...) uses the queuepair aio_context so
>the assumption is that they match.)
>
> Please confirm and add something equivalent into the commit description
> so the assumptions/limitations are clear.
I'll do my best!
Thanks for your reviews,
Phil.
