Re: [PATCH v2 1/3] hw/nvme: support irq(de)assertion with eventfd

[Klaus Jensen <its@irrelevant.dk>]

[-- Attachment #1: Type: text/plain, Size: 2291 bytes --]

On Aug 25 15:47, Jinhao Fan wrote:
> When the new option 'irq-eventfd' is turned on, the IO emulation code
> signals an eventfd when it want to (de)assert an irq. The main loop
> eventfd handler does the actual irq (de)assertion.  This paves the way
> for iothread support since QEMU's interrupt emulation is not thread
> safe.
> 
> Asserting and deasseting irq with eventfd has some performance
> implications. For small queue depth it increases request latency but
> for large queue depth it effectively coalesces irqs.
> 
> Comparision (KIOPS):
> 
> QD            1   4  16  64
> QEMU         38 123 210 329
> irq-eventfd  32 106 240 364
> 
> Signed-off-by: Jinhao Fan <fanjinhao21s@ict.ac.cn>
> ---
>  hw/nvme/ctrl.c | 136 ++++++++++++++++++++++++++++++++++++++++++-------
>  hw/nvme/nvme.h |   4 ++
>  2 files changed, 123 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> index 87aeba0564..6ecf6fafd9 100644
> --- a/hw/nvme/ctrl.c
> +++ b/hw/nvme/ctrl.c
> @@ -1377,8 +1448,25 @@ static void nvme_post_cqes(void *opaque)
>          QTAILQ_INSERT_TAIL(&sq->req_list, req, entry);
>      }
>      if (cq->tail != cq->head) {
> -        if (cq->irq_enabled && !pending) {
> -            n->cq_pending++;
> +        if (cq->irq_enabled) {
> +            if (!pending) {
> +                n->cq_pending++;
> +            }
> +
> +            if (unlikely(cq->first_io_cqe)) {
> +                /*
> +                 * Initilize event notifier when first cqe is posted. For irqfd 
> +                 * support we need to register the MSI message in KVM. We
> +                 * can not do this registration at CQ creation time because
> +                 * Linux's NVMe driver changes the MSI message after CQ creation.
> +                 */
> +                cq->first_io_cqe = false;
> +
> +                if (n->params.irq_eventfd) {
> +                    nvme_init_irq_notifier(n, cq);
> +                }
> +            }

I'm still a bit perplexed by this issue, so I just tried moving
nvme_init_irq_notifier() to the end of nvme_init_cq() and removing this
first_io_cqe thing. I did not observe any particular issues?

What bad behavior did you encounter, it seems to work fine to me?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]