Re: [RFC PATCH v3 1/5] block/nvme: Use an array of EventNotifier

[Stefan Hajnoczi]

On Tue, Aug 18, 2020 at 06:45:05PM +0200, Philippe Mathieu-Daudé wrote:
> In preparation of using multiple IRQ (thus multiple eventfds)
> make BDRVNVMeState::irq_notifier an array (for now of a single
> element, the admin queue notifier).
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  block/nvme.c | 28 ++++++++++++++++++----------
>  1 file changed, 18 insertions(+), 10 deletions(-)

This looks like an intermediate step before using multiple irqs. I think
it makes the code confusing because on one hand INDEX_ADMIN gives the
impression that INDEX_IO() should be used for io queues, while on the
other hand only a single EventNotifier is allocated and we actually
can't use INDEX_IO() yet.

If this intermediate patch is really necessary, please don't use
INDEX_ADMIN. Define a new constant instead:

  /* This driver shares a single MSIX IRQ for the admin and I/O queues */
  #define MSIX_SHARED_IRQ_IDX 0

In the future the array index can be changed to INDEX_ADMIN and
INDEX_IO(n) when there are multiple EventNotifiers.

I think that would make the code clearer.

signature.asc
Description: PGP signature