From: "Yao, Lei A" <lei.a.yao@intel.com>
To: "Burakov, Anatoly" <anatoly.burakov@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>
Cc: "Richardson, Bruce" <bruce.richardson@intel.com>,
"Xu, Qian Q" <qian.q.xu@intel.com>,
"Lu, PeipeiX" <peipeix.lu@intel.com>
Subject: Re: [dpdk-dev] [PATCH] eal: fix circular dependency in EAL proc type detection
Date: Wed, 18 Jul 2018 15:58:29 +0000 [thread overview]
Message-ID: <2DBBFF226F7CF64BAFCA79B681719D953A4F9683@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <be78bcd8da77c58dc43764e562295485f987ea71.1531911034.git.anatoly.burakov@intel.com>
> -----Original Message-----
> From: Burakov, Anatoly
> Sent: Wednesday, July 18, 2018 11:54 AM
> To: dev@dpdk.org
> Cc: Richardson, Bruce <bruce.richardson@intel.com>; Xu, Qian Q
> <qian.q.xu@intel.com>; Yao, Lei A <lei.a.yao@intel.com>; Lu, PeipeiX
> <peipeix.lu@intel.com>
> Subject: [PATCH] eal: fix circular dependency in EAL proc type detection
>
> Currently, we need runtime dir to put all of our runtime info in,
> including the DPDK shared config. However, we use the shared
> config to determine our proc type, and this happens earlier than
> we actually create the config dir and thus can know where to
> place the config file.
>
> Fix this by moving runtime dir creation right after the EAL
> arguments parsing, but before proc type autodetection. Also,
> previously we were creating the config file unconditionally,
> even if we specified no_shconf - fix it by only creating
> the config file if no_shconf is not set.
>
> Fixes: adf1d867361c ("eal: move runtime config file to new location")
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Tested-by: Yao Lei<lei.a.yao@intel.com>
This patch pass the test with simple_mp sample. The secondary process
can recognize itself as "secondary process" even use "--proc-type=auto"
parameter.
> ---
> lib/librte_eal/bsdapp/eal/eal.c | 33 ++++++++++++++++++-------------
> lib/librte_eal/linuxapp/eal/eal.c | 33 ++++++++++++++++++-------------
> 2 files changed, 38 insertions(+), 28 deletions(-)
>
> diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
> index 73cdf07b8..7b399bc9d 100644
> --- a/lib/librte_eal/bsdapp/eal/eal.c
> +++ b/lib/librte_eal/bsdapp/eal/eal.c
> @@ -286,12 +286,17 @@ eal_proc_type_detect(void)
> enum rte_proc_type_t ptype = RTE_PROC_PRIMARY;
> const char *pathname = eal_runtime_config_path();
>
> - /* if we can open the file but not get a write-lock we are a secondary
> - * process. NOTE: if we get a file handle back, we keep that open
> - * and don't close it to prevent a race condition between multiple
> opens */
> - if (((mem_cfg_fd = open(pathname, O_RDWR)) >= 0) &&
> - (fcntl(mem_cfg_fd, F_SETLK, &wr_lock) < 0))
> - ptype = RTE_PROC_SECONDARY;
> + /* if there no shared config, there can be no secondary processes */
> + if (!internal_config.no_shconf) {
> + /* if we can open the file but not get a write-lock we are a
> + * secondary process. NOTE: if we get a file handle back, we
> + * keep that open and don't close it to prevent a race
> condition
> + * between multiple opens.
> + */
> + if (((mem_cfg_fd = open(pathname, O_RDWR)) >= 0) &&
> + (fcntl(mem_cfg_fd, F_SETLK, &wr_lock) < 0))
> + ptype = RTE_PROC_SECONDARY;
> + }
>
> RTE_LOG(INFO, EAL, "Auto-detected process type: %s\n",
> ptype == RTE_PROC_PRIMARY ? "PRIMARY" :
> "SECONDARY");
> @@ -468,6 +473,14 @@ eal_parse_args(int argc, char **argv)
> }
> }
>
> + /* create runtime data directory */
> + if (internal_config.no_shconf == 0 &&
> + eal_create_runtime_dir() < 0) {
> + RTE_LOG(ERR, EAL, "Cannot create runtime directory\n");
> + ret = -1;
> + goto out;
> + }
> +
> if (eal_adjust_config(&internal_config) != 0) {
> ret = -1;
> goto out;
> @@ -600,14 +613,6 @@ rte_eal_init(int argc, char **argv)
> return -1;
> }
>
> - /* create runtime data directory */
> - if (internal_config.no_shconf == 0 &&
> - eal_create_runtime_dir() < 0) {
> - rte_eal_init_alert("Cannot create runtime directory\n");
> - rte_errno = EACCES;
> - return -1;
> - }
> -
> /* FreeBSD always uses legacy memory model */
> internal_config.legacy_mem = true;
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
> index d75ae9dae..d2d5aae80 100644
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -344,12 +344,17 @@ eal_proc_type_detect(void)
> enum rte_proc_type_t ptype = RTE_PROC_PRIMARY;
> const char *pathname = eal_runtime_config_path();
>
> - /* if we can open the file but not get a write-lock we are a secondary
> - * process. NOTE: if we get a file handle back, we keep that open
> - * and don't close it to prevent a race condition between multiple
> opens */
> - if (((mem_cfg_fd = open(pathname, O_RDWR)) >= 0) &&
> - (fcntl(mem_cfg_fd, F_SETLK, &wr_lock) < 0))
> - ptype = RTE_PROC_SECONDARY;
> + /* if there no shared config, there can be no secondary processes */
> + if (!internal_config.no_shconf) {
> + /* if we can open the file but not get a write-lock we are a
> + * secondary process. NOTE: if we get a file handle back, we
> + * keep that open and don't close it to prevent a race
> condition
> + * between multiple opens.
> + */
> + if (((mem_cfg_fd = open(pathname, O_RDWR)) >= 0) &&
> + (fcntl(mem_cfg_fd, F_SETLK, &wr_lock) < 0))
> + ptype = RTE_PROC_SECONDARY;
> + }
>
> RTE_LOG(INFO, EAL, "Auto-detected process type: %s\n",
> ptype == RTE_PROC_PRIMARY ? "PRIMARY" :
> "SECONDARY");
> @@ -692,6 +697,14 @@ eal_parse_args(int argc, char **argv)
> }
> }
>
> + /* create runtime data directory */
> + if (internal_config.no_shconf == 0 &&
> + eal_create_runtime_dir() < 0) {
> + RTE_LOG(ERR, EAL, "Cannot create runtime directory\n");
> + ret = -1;
> + goto out;
> + }
> +
> if (eal_adjust_config(&internal_config) != 0) {
> ret = -1;
> goto out;
> @@ -831,14 +844,6 @@ rte_eal_init(int argc, char **argv)
> return -1;
> }
>
> - /* create runtime data directory */
> - if (internal_config.no_shconf == 0 &&
> - eal_create_runtime_dir() < 0) {
> - rte_eal_init_alert("Cannot create runtime directory\n");
> - rte_errno = EACCES;
> - return -1;
> - }
> -
> if (eal_plugins_init() < 0) {
> rte_eal_init_alert("Cannot init plugins\n");
> rte_errno = EINVAL;
> --
> 2.17.1
next prev parent reply other threads:[~2018-07-18 15:58 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-18 10:53 Anatoly Burakov
2018-07-18 15:58 ` Yao, Lei A [this message]
2018-07-19 10:17 ` Thomas Monjalon
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=2DBBFF226F7CF64BAFCA79B681719D953A4F9683@shsmsx102.ccr.corp.intel.com \
--to=lei.a.yao@intel.com \
--cc=anatoly.burakov@intel.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=peipeix.lu@intel.com \
--cc=qian.q.xu@intel.com \
/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).