[1/3] eal: remove useless checks for already probed device
diff mbox series

Message ID 20190302024253.15594-2-thomas@monjalon.net
State New
Delegated to: Thomas Monjalon
Headers show
Series
  • fix error path of multi-process probe
Related show

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/intel-Performance-Testing success Performance Testing PASS
ci/mellanox-Performance-Testing success Performance Testing PASS

Commit Message

Thomas Monjalon March 2, 2019, 2:42 a.m. UTC
The function eal_dev_hotplug_request_to_secondary() never returns
-EEXIST result. The case of already probed device is filtered out.

The test in __handle_secondary_request() was always true.
The test in rte_dev_probe() was never true, and that's fine not
returning -EEXIST if device is already attached in secondary processes.

Fixes: 494db286f37d ("eal: fix multi-process hotplug if attached in secondary")
Cc: dariusz.stojaczyk@intel.com
Cc: qi.z.zhang@intel.com
Cc: stable@dpdk.org

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 lib/librte_eal/common/eal_common_dev.c | 9 +--------
 lib/librte_eal/common/hotplug_mp.c     | 3 +--
 2 files changed, 2 insertions(+), 10 deletions(-)

Comments

Qi Zhang March 13, 2019, 1:46 p.m. UTC | #1
Hi:

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Saturday, March 2, 2019 10:43 AM
> To: dev@dpdk.org
> Cc: Stojaczyk, Dariusz <dariusz.stojaczyk@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; stable@dpdk.org
> Subject: [PATCH 1/3] eal: remove useless checks for already probed device
> 
> The function eal_dev_hotplug_request_to_secondary() never returns -EEXIST
> result. The case of already probed device is filtered out.
> 
> The test in __handle_secondary_request() was always true.
> The test in rte_dev_probe() was never true, and that's fine not returning -EEXIST
> if device is already attached in secondary processes.

I didn't get this.
eal_dev_hotplug_request_to_secondary() never return -EEXIST, but req->result could be -EEXIST.

This happens when secondary try to attach an already attached device 
(__handle_primary_request --> local_dev_probe --> dev->bus->plug )


> 
> Fixes: 494db286f37d ("eal: fix multi-process hotplug if attached in secondary")
> Cc: dariusz.stojaczyk@intel.com
> Cc: qi.z.zhang@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>  lib/librte_eal/common/eal_common_dev.c | 9 +--------
>  lib/librte_eal/common/hotplug_mp.c     | 3 +--
>  2 files changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/librte_eal/common/eal_common_dev.c
> b/lib/librte_eal/common/eal_common_dev.c
> index fd7f5ca7d5..048c0b025f 100644
> --- a/lib/librte_eal/common/eal_common_dev.c
> +++ b/lib/librte_eal/common/eal_common_dev.c
> @@ -247,18 +247,11 @@ rte_dev_probe(const char *devargs)
>  		goto rollback;
>  	}
> 
> -	/**
> -	 * if any secondary failed to attach, we need to consider if rollback
> -	 * is necessary.
> -	 */
> +	/* if any secondary failed to attach, need to rollback. */
>  	if (req.result != 0) {
>  		RTE_LOG(ERR, EAL,
>  			"Failed to attach device on secondary process\n");
>  		ret = req.result;
> -
> -		/* for -EEXIST, we don't need to rollback. */
> -		if (ret == -EEXIST)
> -			return ret;
>  		goto rollback;
>  	}
> 
> diff --git a/lib/librte_eal/common/hotplug_mp.c
> b/lib/librte_eal/common/hotplug_mp.c
> index 4052a5c7fb..94bd1d896e 100644
> --- a/lib/librte_eal/common/hotplug_mp.c
> +++ b/lib/librte_eal/common/hotplug_mp.c
> @@ -110,8 +110,7 @@ __handle_secondary_request(void *param)
>  		if (tmp_req.result != 0) {
>  			ret = tmp_req.result;
>  			RTE_LOG(ERR, EAL, "Failed to hotplug add device on
> secondary\n");
> -			if (ret != -EEXIST)
> -				goto rollback;
> +			goto rollback;
>  		}
>  	} else if (req->t == EAL_DEV_REQ_TYPE_DETACH) {
>  		ret = rte_devargs_parse(&da, req->devargs);
> --
> 2.20.1

Patch
diff mbox series

diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index fd7f5ca7d5..048c0b025f 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -247,18 +247,11 @@  rte_dev_probe(const char *devargs)
 		goto rollback;
 	}
 
-	/**
-	 * if any secondary failed to attach, we need to consider if rollback
-	 * is necessary.
-	 */
+	/* if any secondary failed to attach, need to rollback. */
 	if (req.result != 0) {
 		RTE_LOG(ERR, EAL,
 			"Failed to attach device on secondary process\n");
 		ret = req.result;
-
-		/* for -EEXIST, we don't need to rollback. */
-		if (ret == -EEXIST)
-			return ret;
 		goto rollback;
 	}
 
diff --git a/lib/librte_eal/common/hotplug_mp.c b/lib/librte_eal/common/hotplug_mp.c
index 4052a5c7fb..94bd1d896e 100644
--- a/lib/librte_eal/common/hotplug_mp.c
+++ b/lib/librte_eal/common/hotplug_mp.c
@@ -110,8 +110,7 @@  __handle_secondary_request(void *param)
 		if (tmp_req.result != 0) {
 			ret = tmp_req.result;
 			RTE_LOG(ERR, EAL, "Failed to hotplug add device on secondary\n");
-			if (ret != -EEXIST)
-				goto rollback;
+			goto rollback;
 		}
 	} else if (req->t == EAL_DEV_REQ_TYPE_DETACH) {
 		ret = rte_devargs_parse(&da, req->devargs);