DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Zhang, Qi Z" <qi.z.zhang@intel.com>
To: Thomas Monjalon <thomas@monjalon.net>, "dev@dpdk.org" <dev@dpdk.org>
Cc: "Stojaczyk, Dariusz" <dariusz.stojaczyk@intel.com>,
	"stable@dpdk.org" <stable@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH 1/3] eal: remove useless checks for already probed device
Date: Wed, 13 Mar 2019 13:46:01 +0000	[thread overview]
Message-ID: <039ED4275CED7440929022BC67E706115334BEC4@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <20190302024253.15594-2-thomas@monjalon.net>

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

  reply	other threads:[~2019-03-13 13:46 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-02  2:42 [dpdk-dev] [PATCH 0/3] fix error path of multi-process probe Thomas Monjalon
2019-03-02  2:42 ` [dpdk-dev] [PATCH 1/3] eal: remove useless checks for already probed device Thomas Monjalon
2019-03-13 13:46   ` Zhang, Qi Z [this message]
2023-06-11  2:52     ` Stephen Hemminger
2019-03-02  2:42 ` [dpdk-dev] [PATCH 2/3] eal: remove error logs " Thomas Monjalon
2019-03-02  2:42 ` [dpdk-dev] [PATCH 3/3] eal: fix multi-process probe failure handling Thomas Monjalon
2019-03-14  7:15   ` Zhang, Qi Z
2019-03-14  7:15     ` Zhang, Qi Z
2023-06-14 19:39 ` [dpdk-dev] [PATCH 0/3] fix error path of multi-process probe Stephen Hemminger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=039ED4275CED7440929022BC67E706115334BEC4@SHSMSX103.ccr.corp.intel.com \
    --to=qi.z.zhang@intel.com \
    --cc=dariusz.stojaczyk@intel.com \
    --cc=dev@dpdk.org \
    --cc=stable@dpdk.org \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).