[v2] raw/ntb: add check for disabling interrupt in dev close ops

Message ID 20230628091218.32292-1-junfeng.guo@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series [v2] raw/ntb: add check for disabling interrupt in dev close ops |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/github-robot: build success github build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-unit-testing success Testing PASS
ci/iol-aarch-unit-testing success Testing PASS
ci/iol-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/intel-Functional success Functional PASS

Commit Message

Junfeng Guo June 28, 2023, 9:12 a.m. UTC
  During EAL cleanup stage, all bus devices are cleaned up properly.
In the meantime, the ntb example app will also do the device cleanup
process, which may call the dev ops '*dev->dev_ops->dev_close' twice.

If this dev ops for ntb was called twice, the interrupt handle for
EAL will be disabled twice and will lead to error for the seconde
time. Like this: "EAL: Error disabling MSI-X interrupts for fd xx"

Thus, this patch added the check process for disabling interrupt in
dev_close ops, to ensure that interrupt only be disabled once.

Fixes: 1cab1a40ea9b ("bus: cleanup devices on shutdown")
Cc: stable@dpdk.org

Signed-off-by: Junfeng Guo <junfeng.guo@intel.com>
---
 drivers/raw/ntb/ntb.c | 5 +++++
 1 file changed, 5 insertions(+)
  

Comments

Ling, WeiX June 29, 2023, 8:03 a.m. UTC | #1
> -----Original Message-----
> From: Junfeng Guo <junfeng.guo@intel.com>
> Sent: Wednesday, June 28, 2023 5:12 PM
> To: Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org; He, Xingguang
> <xingguang.he@intel.com>; Laatz, Kevin <kevin.laatz@intel.com>; Guo,
> Junfeng <junfeng.guo@intel.com>
> Subject: [PATCH v2] raw/ntb: add check for disabling interrupt in dev close
> ops
> 
> During EAL cleanup stage, all bus devices are cleaned up properly.
> In the meantime, the ntb example app will also do the device cleanup
> process, which may call the dev ops '*dev->dev_ops->dev_close' twice.
> 
> If this dev ops for ntb was called twice, the interrupt handle for EAL will be
> disabled twice and will lead to error for the seconde time. Like this: "EAL:
> Error disabling MSI-X interrupts for fd xx"
> 
> Thus, this patch added the check process for disabling interrupt in dev_close
> ops, to ensure that interrupt only be disabled once.
> 
> Fixes: 1cab1a40ea9b ("bus: cleanup devices on shutdown")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Junfeng Guo <junfeng.guo@intel.com>
> ---
>  drivers/raw/ntb/ntb.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 

Tested-by: Wei Ling <weix.ling@intel.com>
  
Jingjing Wu July 3, 2023, 6:53 a.m. UTC | #2
> -----Original Message-----
> From: Guo, Junfeng <junfeng.guo@intel.com>
> Sent: Wednesday, June 28, 2023 5:12 PM
> To: Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org; He, Xingguang <xingguang.he@intel.com>; Laatz, Kevin
> <kevin.laatz@intel.com>; Guo, Junfeng <junfeng.guo@intel.com>
> Subject: [PATCH v2] raw/ntb: add check for disabling interrupt in dev close ops
> 
> During EAL cleanup stage, all bus devices are cleaned up properly.
> In the meantime, the ntb example app will also do the device cleanup
> process, which may call the dev ops '*dev->dev_ops->dev_close' twice.
> 
> If this dev ops for ntb was called twice, the interrupt handle for
> EAL will be disabled twice and will lead to error for the seconde
> time. Like this: "EAL: Error disabling MSI-X interrupts for fd xx"
> 
> Thus, this patch added the check process for disabling interrupt in
> dev_close ops, to ensure that interrupt only be disabled once.
> 
> Fixes: 1cab1a40ea9b ("bus: cleanup devices on shutdown")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Junfeng Guo <junfeng.guo@intel.com>

Acked-by: Jingjing Wu <jingjing.wu@intel.com>
  
Thomas Monjalon July 3, 2023, 3:44 p.m. UTC | #3
03/07/2023 08:53, Wu, Jingjing:
> 
> > -----Original Message-----
> > From: Guo, Junfeng <junfeng.guo@intel.com>
> > Sent: Wednesday, June 28, 2023 5:12 PM
> > To: Wu, Jingjing <jingjing.wu@intel.com>
> > Cc: dev@dpdk.org; stable@dpdk.org; He, Xingguang <xingguang.he@intel.com>; Laatz, Kevin
> > <kevin.laatz@intel.com>; Guo, Junfeng <junfeng.guo@intel.com>
> > Subject: [PATCH v2] raw/ntb: add check for disabling interrupt in dev close ops
> > 
> > During EAL cleanup stage, all bus devices are cleaned up properly.
> > In the meantime, the ntb example app will also do the device cleanup
> > process, which may call the dev ops '*dev->dev_ops->dev_close' twice.
> > 
> > If this dev ops for ntb was called twice, the interrupt handle for
> > EAL will be disabled twice and will lead to error for the seconde
> > time. Like this: "EAL: Error disabling MSI-X interrupts for fd xx"
> > 
> > Thus, this patch added the check process for disabling interrupt in
> > dev_close ops, to ensure that interrupt only be disabled once.
> > 
> > Fixes: 1cab1a40ea9b ("bus: cleanup devices on shutdown")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Junfeng Guo <junfeng.guo@intel.com>
> 
> Acked-by: Jingjing Wu <jingjing.wu@intel.com>

Applied, thanks.
  

Patch

diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c
index 76e98fe515..0ed4c14592 100644
--- a/drivers/raw/ntb/ntb.c
+++ b/drivers/raw/ntb/ntb.c
@@ -1045,6 +1045,11 @@  ntb_dev_close(struct rte_rawdev *dev)
 	hw->queue_pairs = 0;
 
 	intr_handle = hw->pci_dev->intr_handle;
+	/* Disable interrupt only once */
+	if (!rte_intr_nb_efd_get(intr_handle) &&
+	    !rte_intr_max_intr_get(intr_handle))
+		return 0;
+
 	/* Clean datapath event and vec mapping */
 	rte_intr_efd_disable(intr_handle);
 	rte_intr_vec_list_free(intr_handle);