On 18-Jul-19 9:46 AM, Nithin Dabilpuram wrote:
Add new ack interrupt API to avoid using
VFIO_IRQ_SET_ACTION_TRIGGER(rte_intr_enable()) for
acking interrupt purpose for VFIO based interrupt handlers.
This implementation is specific to Linux.

Using rte_intr_enable() for acking interrupt has below issues

  * Time consuming to do for every interrupt received as it will
    free_irq() followed by request_irq() and all other initializations
  * A race condition because of a window between free_irq() and
    request_irq() with packet reception still on and device still
    enabled and would throw warning messages like below.
    [158764.159833] do_IRQ: 9.34 No irq handler for vector

In this patch, rte_intr_ack() is a no-op for VFIO_MSIX/VFIO_MSI interrupts
as they are edge triggered and kernel would not mask the interrupt before
delivering the event to userspace and we don't need to ack.

Signed-off-by: Nithin Dabilpuram <ndabilpu...@marvell.com>
Signed-off-by: Jerin Jacob <jer...@marvell.com>
Tested-by: Shahed Shaikh <shsha...@marvell.com>
---

<snip>

   */
  int rte_intr_disable(const struct rte_intr_handle *intr_handle);
+/**
+ * It acks an interrupt raised for the specified handle.
+ *
+ * Call this function to ack an interrupt from interrupt
+ * handler either from application or driver, so that
+ * new interrupts are raised.
+ *
+ * @note PMD generally calls this function at the end of its IRQ callback.
+ *    Internally, it unmasks the interrupt if possible. For INTx, unmasking
+ *    is required as the interrupt is auto-masked prior to invoking
+ *    callback. For MSI/MSI-X, unmasking is typically not needed as the
+ *    interrupt is not auto-masked. In fact, for interrupt handle types
+ *    VFIO_MSIX and VFIO_MSI, this function is no-op.

I've added this comment in previous revision's discussion, but i'll copy it here as well.

Does the user of this API even cares about these details? I would think that it would be easier to just mandate calling this function at the end of each interrupt callback, regardless of which interrupt mode is used.

IMO internal details (about no-ops and such) are better explained in the implementation, not in the public-facing API header.

--
Thanks,
Anatoly

Reply via email to