Re: [dpdk-dev] [PATCH] eal: fix circular dependency in EAL proc type detection

["Yao, Lei A" <lei.a.yao@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