nvme_irq_assert() only does useful work when cq->irq_enabled is true.
nvme_irq_deassert() only works for pin-based interrupts. Avoid calls
into these functions if we are sure they will not do useful work.

This will be most useful when we use eventfd to send interrupts. We
can avoid the unnecessary overhead of signalling eventfd.

Signed-off-by: Jinhao Fan <fanjinhao21s@ict.ac.cn>
---
 hw/nvme/ctrl.c | 40 ++++++++++++++++++++++------------------
 1 file changed, 22 insertions(+), 18 deletions(-)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 87aeba0564..bd3350d7e0 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -1377,11 +1377,13 @@ 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++;
+            }
 
-        nvme_irq_assert(n, cq);
+            nvme_irq_assert(n, cq);
+        }
     }
 }
 
@@ -4244,12 +4246,11 @@ static void nvme_cq_notifier(EventNotifier *e)
 
     nvme_update_cq_head(cq);
 
-    if (cq->tail == cq->head) {
-        if (cq->irq_enabled) {
-            n->cq_pending--;
+    if (cq->irq_enabled && cq->tail == cq->head) {
+        n->cq_pending--;
+        if (!msix_enabled(&n->parent_obj)) {
+            nvme_irq_deassert(n, cq);
         }
-
-        nvme_irq_deassert(n, cq);
     }
 
     nvme_post_cqes(cq);
@@ -4730,11 +4731,15 @@ static uint16_t nvme_del_cq(NvmeCtrl *n, NvmeRequest *req)
         return NVME_INVALID_QUEUE_DEL;
     }
 
-    if (cq->irq_enabled && cq->tail != cq->head) {
-        n->cq_pending--;
-    }
+    if (cq->irq_enabled) {
+        if (cq->tail != cq->head) {
+            n->cq_pending--;
+        }
 
-    nvme_irq_deassert(n, cq);
+        if (!msix_enabled(&n->parent_obj)) {
+            nvme_irq_deassert(n, cq);
+        }
+    }
     trace_pci_nvme_del_cq(qid);
     nvme_free_cq(cq, n);
     return NVME_SUCCESS;
@@ -6918,12 +6923,11 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, int val)
             timer_mod(cq->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 500);
         }
 
-        if (cq->tail == cq->head) {
-            if (cq->irq_enabled) {
-                n->cq_pending--;
+        if (cq->irq_enabled && cq->tail == cq->head) {
+            n->cq_pending--;
+            if (!msix_enabled(&n->parent_obj)) {
+                nvme_irq_deassert(n, cq);
             }
-
-            nvme_irq_deassert(n, cq);
         }
     } else {
         /* Submission queue doorbell write */
-- 
2.25.1

On Thu, 11 Aug 2022 at 11:38, Jinhao Fan <fanjinhao21s@ict.ac.cn> wrote:
>
> nvme_irq_assert() only does useful work when cq->irq_enabled is true.
> nvme_irq_deassert() only works for pin-based interrupts. Avoid calls
> into these functions if we are sure they will not do useful work.
>
> This will be most useful when we use eventfd to send interrupts. We
> can avoid the unnecessary overhead of signalling eventfd.
>
> Signed-off-by: Jinhao Fan <fanjinhao21s@ict.ac.cn>
> ---
>  hw/nvme/ctrl.c | 40 ++++++++++++++++++++++------------------
>  1 file changed, 22 insertions(+), 18 deletions(-)

There is code duplication and nvme_irq_assert/deassert() check
->irq_enabled and msix_enabled() again.

Can the logic be moved into assert()/deassert() so callers don't need
to duplicate the checks?

(I assume the optimization is that eventfd syscalls are avoided, not
that the function call is avoided.)

Stefan

at 11:24 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:

> Can the logic be moved into assert()/deassert() so callers don't need
> to duplicate the checks?
> 
> (I assume the optimization is that eventfd syscalls are avoided, not
> that the function call is avoided.)

I guess I can move the eventfd syscall into assert()/deassert().