Re: [dpdk-dev] [PATCH v2 2/3] eal: add ack interrupt API

["Hyong Youb Kim (hyonkim)" <hyonkim@cisco.com>]

> -----Original Message-----
> From: Nithin Dabilpuram <ndabilpuram@marvell.com>
> Sent: Wednesday, July 17, 2019 9:44 PM
> To: Hyong Youb Kim (hyonkim) <hyonkim@cisco.com>; David Marchand
> <david.marchand@redhat.com>; Thomas Monjalon
> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>; Bruce
> Richardson <bruce.richardson@intel.com>
> Cc: jerinj@marvell.com; John Daley (johndale) <johndale@cisco.com>;
> Shahed Shaikh <shshaikh@marvell.com>; dev@dpdk.org; Nithin Dabilpuram
> <ndabilpuram@marvell.com>
> Subject: [PATCH v2 2/3] eal: add ack interrupt API
> 
> 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 <ndabilpuram@marvell.com>
> Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> ---
> v2:
> * No change
> 
>  lib/librte_eal/common/include/rte_interrupts.h | 22 +++++++
>  lib/librte_eal/freebsd/eal/eal_interrupts.c    |  9 +++
>  lib/librte_eal/linux/eal/eal_interrupts.c      | 81
> ++++++++++++++++++++++++++
>  lib/librte_eal/rte_eal_version.map             |  1 +
>  4 files changed, 113 insertions(+)
> 
> diff --git a/lib/librte_eal/common/include/rte_interrupts.h
> b/lib/librte_eal/common/include/rte_interrupts.h
> index c1e912c..93b31cd 100644
> --- a/lib/librte_eal/common/include/rte_interrupts.h
> +++ b/lib/librte_eal/common/include/rte_interrupts.h
> @@ -118,6 +118,28 @@ int rte_intr_enable(const struct rte_intr_handle
> *intr_handle);
>   */
>  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 For interrupt handle types VFIO_MSIX and VFIO_MSI,
> + *    this function is a no-op and returns success without
> + *    changing anything as kernel doesn't expect
> + *    them to be acked.
> + *
[...]

Shouldn't we explain that this really is "unmask" but named "ack" because
of x and y, and that it is expected at end of INTx handler? Ack does
not have a well-defined meaning, whereas everyone knows what unmask
means..

[...]

Thanks.
-Hyong