Re: [dpdk-dev] [EXT] [PATCH v4 1/1] eal/interrupts: add synchronous wrapper around unregister

[Harman Kalra <hkalra@marvell.com>]

> Avoid race with unregister interrupt handler if interrupt source has some
> active callbacks at the moment, use wrapper around
> rte_intr_callback_unregister() to check for -EAGAIN return value and to
> loop until rte_intr_callback_unregister() succeeds.
> 

Hi Renata,
   Thanks for the clarification, changes looks good to me.

Acked-by: Harman Kalra <hkalra@marvell.com>

> Signed-off-by: Renata Saiakhova <Renata.Saiakhova@ekinops.com>
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  drivers/bus/pci/linux/pci_vfio.c        |  2 +-
>  lib/librte_eal/freebsd/eal_interrupts.c | 12 ++++++++++++
> lib/librte_eal/include/rte_interrupts.h | 25 +++++++++++++++++++++++++
>  lib/librte_eal/linux/eal_interrupts.c   | 12 ++++++++++++
>  lib/librte_eal/version.map              |  1 +
>  5 files changed, 51 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/bus/pci/linux/pci_vfio.c
> b/drivers/bus/pci/linux/pci_vfio.c
> index e3f7b6abe..c14f7f8c4 100644
> --- a/drivers/bus/pci/linux/pci_vfio.c
> +++ b/drivers/bus/pci/linux/pci_vfio.c
> @@ -414,7 +414,7 @@ pci_vfio_disable_notifier(struct rte_pci_device
> *dev)
>  		return -1;
>  	}
> 
> -	ret = rte_intr_callback_unregister(&dev->vfio_req_intr_handle,
> +	ret = rte_intr_callback_unregister_sync(&dev-
> >vfio_req_intr_handle,
>  					   pci_vfio_req_handler,
>  					   (void *)&dev->device);
>  	if (ret < 0) {
> diff --git a/lib/librte_eal/freebsd/eal_interrupts.c
> b/lib/librte_eal/freebsd/eal_interrupts.c
> index 72eeacbc1..86810845f 100644
> --- a/lib/librte_eal/freebsd/eal_interrupts.c
> +++ b/lib/librte_eal/freebsd/eal_interrupts.c
> @@ -345,6 +345,18 @@ rte_intr_callback_unregister(const struct
> rte_intr_handle *intr_handle,
>  	return ret;
>  }
> 
> +int
> +rte_intr_callback_unregister_sync(const struct rte_intr_handle
> *intr_handle,
> +		rte_intr_callback_fn cb_fn, void *cb_arg) {
> +	int ret = 0;
> +
> +	while ((ret = rte_intr_callback_unregister(intr_handle, cb_fn,
> cb_arg)) == -EAGAIN)
> +		rte_pause();
> +
> +	return ret;
> +}
> +
>  int
>  rte_intr_enable(const struct rte_intr_handle *intr_handle)  { diff --git
> a/lib/librte_eal/include/rte_interrupts.h
> b/lib/librte_eal/include/rte_interrupts.h
> index e3b406abc..cc3bf45d8 100644
> --- a/lib/librte_eal/include/rte_interrupts.h
> +++ b/lib/librte_eal/include/rte_interrupts.h
> @@ -94,6 +94,31 @@ rte_intr_callback_unregister_pending(const struct
> rte_intr_handle *intr_handle,
>  				rte_intr_callback_fn cb_fn, void *cb_arg,
>  				rte_intr_unregister_callback_fn ucb_fn);
> 
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Loop until rte_intr_callback_unregister() succeeds.
> + * After a call to this function,
> + * the callback provided by the specified interrupt handle is unregistered.
> + *
> + * @param intr_handle
> + *  pointer to the interrupt handle.
> + * @param cb
> + *  callback address.
> + * @param cb_arg
> + *  address of parameter for callback, (void *)-1 means to remove all
> + *  registered which has the same callback address.
> + *
> + * @return
> + *  - On success, return the number of callback entities removed.
> + *  - On failure, a negative value.
> + */
> +__rte_experimental
> +int
> +rte_intr_callback_unregister_sync(const struct rte_intr_handle
> *intr_handle,
> +				rte_intr_callback_fn cb, void *cb_arg);
> +
>  /**
>   * It enables the interrupt for the specified handle.
>   *
> diff --git a/lib/librte_eal/linux/eal_interrupts.c
> b/lib/librte_eal/linux/eal_interrupts.c
> index 1dd994bd1..22b3b7bcd 100644
> --- a/lib/librte_eal/linux/eal_interrupts.c
> +++ b/lib/librte_eal/linux/eal_interrupts.c
> @@ -662,6 +662,18 @@ rte_intr_callback_unregister(const struct
> rte_intr_handle *intr_handle,
>  	return ret;
>  }
> 
> +int
> +rte_intr_callback_unregister_sync(const struct rte_intr_handle
> *intr_handle,
> +			rte_intr_callback_fn cb_fn, void *cb_arg) {
> +	int ret = 0;
> +
> +	while ((ret = rte_intr_callback_unregister(intr_handle, cb_fn,
> cb_arg)) == -EAGAIN)
> +		rte_pause();
> +
> +	return ret;
> +}
> +
>  int
>  rte_intr_enable(const struct rte_intr_handle *intr_handle)  { diff --git
> a/lib/librte_eal/version.map b/lib/librte_eal/version.map index
> fce90a112..56caa9cc9 100644
> --- a/lib/librte_eal/version.map
> +++ b/lib/librte_eal/version.map
> @@ -318,6 +318,7 @@ EXPERIMENTAL {
>  	rte_fbarray_find_rev_biggest_free;
>  	rte_fbarray_find_rev_biggest_used;
>  	rte_intr_callback_unregister_pending;
> +	rte_intr_callback_unregister_sync;
>  	rte_realloc_socket;
> 
>  	# added in 19.08
> --
> 2.17.2