| Message ID | 20190302024253.15594-2-thomas@monjalon.net (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Thomas Monjalon |
| Headers | show |
| Series | fix error path of multi-process probe | expand |
| 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 |
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
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);
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(-)