[v2,2/2] bus/pci: fix secondary process save 'FD' problem

Message ID 20240129092231.3531217-3-chaoyong.he@corigine.com (mailing list archive)
State Superseded
Delegated to: Thomas Monjalon
Headers
Series fix secondary process PCI UIO resource problem |

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/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-abi-testing success Testing PASS
ci/github-robot: build success github build: passed
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS

Commit Message

Chaoyong He Jan. 29, 2024, 9:22 a.m. UTC
  From: Zerun Fu <zerun.fu@corigine.com>

In the previous logic the 'fd' was only saved in the primary process,
but for some devices this value is also used in the secondary logic.

For example, the call of 'rte_pci_find_ext_capability()' will fail in
the secondary process.

Fix this problem by getting and saving the value of 'fd' also in the
secondary process logic.

Fixes: 9b957f378abf ("pci: merge uio functions for linux and bsd")
Cc: mukawa@igel.co.jp
Cc: stable@dpdk.org

Signed-off-by: Zerun Fu <zerun.fu@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
---
 drivers/bus/pci/linux/pci_uio.c  | 5 ++++-
 drivers/bus/pci/pci_common_uio.c | 8 ++++----
 2 files changed, 8 insertions(+), 5 deletions(-)
  

Comments

fengchengwen Jan. 30, 2024, 6:16 a.m. UTC | #1
Hi,

The pci_uio_alloc_resource in drivers/bus/pci/bsd should also modified.

How about add subfunction: pci_uio_init_intr() which includes access/rte_intr_fd_set/rte_intr_fd_get/rte_intr_type_set?

Thanks

On 2024/1/29 17:22, Chaoyong He wrote:
> From: Zerun Fu <zerun.fu@corigine.com>
> 
> In the previous logic the 'fd' was only saved in the primary process,
> but for some devices this value is also used in the secondary logic.
> 
> For example, the call of 'rte_pci_find_ext_capability()' will fail in
> the secondary process.
> 
> Fix this problem by getting and saving the value of 'fd' also in the
> secondary process logic.
> 
> Fixes: 9b957f378abf ("pci: merge uio functions for linux and bsd")
> Cc: mukawa@igel.co.jp
> Cc: stable@dpdk.org
> 
> Signed-off-by: Zerun Fu <zerun.fu@corigine.com>
> Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
> Reviewed-by: Long Wu <long.wu@corigine.com>
> Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
> ---
>  drivers/bus/pci/linux/pci_uio.c  | 5 ++++-
>  drivers/bus/pci/pci_common_uio.c | 8 ++++----
>  2 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/bus/pci/linux/pci_uio.c b/drivers/bus/pci/linux/pci_uio.c
> index 97d740dfe5..6680e42efb 100644
> --- a/drivers/bus/pci/linux/pci_uio.c
> +++ b/drivers/bus/pci/linux/pci_uio.c
> @@ -237,7 +237,7 @@ pci_uio_alloc_resource(struct rte_pci_device *dev,
>  	}
>  	snprintf(devname, sizeof(devname), "/dev/uio%u", uio_num);
>  
> -	/* save fd if in primary process */
> +	/* save fd */
>  	fd = open(devname, O_RDWR);
>  	if (fd < 0) {
>  		RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
> @@ -261,6 +261,9 @@ pci_uio_alloc_resource(struct rte_pci_device *dev,
>  	if (rte_intr_dev_fd_set(dev->intr_handle, uio_cfg_fd))
>  		goto error;
>  
> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> +		return 0;
> +
>  	if (dev->kdrv == RTE_PCI_KDRV_IGB_UIO) {
>  		if (rte_intr_type_set(dev->intr_handle, RTE_INTR_HANDLE_UIO))
>  			goto error;
> diff --git a/drivers/bus/pci/pci_common_uio.c b/drivers/bus/pci/pci_common_uio.c
> index fcd8a49daf..b6f79b067d 100644
> --- a/drivers/bus/pci/pci_common_uio.c
> +++ b/drivers/bus/pci/pci_common_uio.c
> @@ -122,15 +122,15 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>  	if (rte_intr_dev_fd_set(dev->intr_handle, -1))
>  		return -1;
>  
> -	/* secondary processes - use already recorded details */
> -	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> -		return pci_uio_map_secondary(dev);
> -
>  	/* allocate uio resource */
>  	ret = pci_uio_alloc_resource(dev, &uio_res);
>  	if (ret)
>  		return ret;
>  
> +	/* secondary processes - use already recorded details */
> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> +		return pci_uio_map_secondary(dev);
> +
>  	/* Map all BARs */
>  	for (i = 0; i != PCI_MAX_RESOURCE; i++) {
>  		/* skip empty BAR */
>
  
Anatoly Burakov March 14, 2024, 11:01 a.m. UTC | #2
On 1/29/2024 10:22 AM, Chaoyong He wrote:
> From: Zerun Fu <zerun.fu@corigine.com>
> 
> In the previous logic the 'fd' was only saved in the primary process,
> but for some devices this value is also used in the secondary logic.
> 
> For example, the call of 'rte_pci_find_ext_capability()' will fail in
> the secondary process.
> 
> Fix this problem by getting and saving the value of 'fd' also in the
> secondary process logic.
> 
> Fixes: 9b957f378abf ("pci: merge uio functions for linux and bsd")
> Cc: mukawa@igel.co.jp
> Cc: stable@dpdk.org
> 
> Signed-off-by: Zerun Fu <zerun.fu@corigine.com>
> Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
> Reviewed-by: Long Wu <long.wu@corigine.com>
> Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
> ---
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
  

Patch

diff --git a/drivers/bus/pci/linux/pci_uio.c b/drivers/bus/pci/linux/pci_uio.c
index 97d740dfe5..6680e42efb 100644
--- a/drivers/bus/pci/linux/pci_uio.c
+++ b/drivers/bus/pci/linux/pci_uio.c
@@ -237,7 +237,7 @@  pci_uio_alloc_resource(struct rte_pci_device *dev,
 	}
 	snprintf(devname, sizeof(devname), "/dev/uio%u", uio_num);
 
-	/* save fd if in primary process */
+	/* save fd */
 	fd = open(devname, O_RDWR);
 	if (fd < 0) {
 		RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
@@ -261,6 +261,9 @@  pci_uio_alloc_resource(struct rte_pci_device *dev,
 	if (rte_intr_dev_fd_set(dev->intr_handle, uio_cfg_fd))
 		goto error;
 
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return 0;
+
 	if (dev->kdrv == RTE_PCI_KDRV_IGB_UIO) {
 		if (rte_intr_type_set(dev->intr_handle, RTE_INTR_HANDLE_UIO))
 			goto error;
diff --git a/drivers/bus/pci/pci_common_uio.c b/drivers/bus/pci/pci_common_uio.c
index fcd8a49daf..b6f79b067d 100644
--- a/drivers/bus/pci/pci_common_uio.c
+++ b/drivers/bus/pci/pci_common_uio.c
@@ -122,15 +122,15 @@  pci_uio_map_resource(struct rte_pci_device *dev)
 	if (rte_intr_dev_fd_set(dev->intr_handle, -1))
 		return -1;
 
-	/* secondary processes - use already recorded details */
-	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
-		return pci_uio_map_secondary(dev);
-
 	/* allocate uio resource */
 	ret = pci_uio_alloc_resource(dev, &uio_res);
 	if (ret)
 		return ret;
 
+	/* secondary processes - use already recorded details */
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return pci_uio_map_secondary(dev);
+
 	/* Map all BARs */
 	for (i = 0; i != PCI_MAX_RESOURCE; i++) {
 		/* skip empty BAR */