[dpdk-dev,v3] eal: restrict cores detection

Message ID 1472693507-11369-1-git-send-email-jianfeng.tan@intel.com
State Changes Requested, archived
Delegated to: Thomas Monjalon
Headers show

Commit Message

Jianfeng Tan Sept. 1, 2016, 1:31 a.m.
This patch uses pthread_getaffinity_np() to narrow down detected
cores before parsing coremask (-c), corelist (-l), and coremap
(--lcores).

The purpose of this patch is to leave out these core related options
when DPDK applications are deployed under container env, so that
users only specify core restriction as starting the instance.

Note: previously, some users are using isolated CPUs, which could
be excluded by default. Please add commands like taskset to use
those cores.

Test example:
$ taskset 0xc0000 ./examples/helloworld/build/helloworld -m 1024

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
---
v3:
  - Choose a more descriptive variable name, and remove comments
    as suggested by Stephen Hemminger.
v2:
  - Make it as default instead of adding the new options.
 lib/librte_eal/common/eal_common_lcore.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Bruce Richardson Sept. 2, 2016, 4:53 p.m. | #1
On Thu, Sep 01, 2016 at 01:31:47AM +0000, Jianfeng Tan wrote:
> This patch uses pthread_getaffinity_np() to narrow down detected
> cores before parsing coremask (-c), corelist (-l), and coremap
> (--lcores).
> 
> The purpose of this patch is to leave out these core related options
> when DPDK applications are deployed under container env, so that
> users only specify core restriction as starting the instance.
> 
> Note: previously, some users are using isolated CPUs, which could
> be excluded by default. Please add commands like taskset to use
> those cores.
> 
> Test example:
> $ taskset 0xc0000 ./examples/helloworld/build/helloworld -m 1024
> 

So, to be clear, does this patch mean that DPDK cannot use isolated cores
any more unless you explicitly run the app using taskset?
Is so, NAK, since isolating cores has been part of standard DPDK setup since
the first versions, and I don't believe that we should break that behaviour.

/Bruce
Thomas Monjalon Sept. 16, 2016, 2:02 p.m. | #2
2016-09-01 01:31, Jianfeng Tan:
> This patch uses pthread_getaffinity_np() to narrow down detected
> cores before parsing coremask (-c), corelist (-l), and coremap
> (--lcores).
> 
> The purpose of this patch is to leave out these core related options
> when DPDK applications are deployed under container env, so that
> users only specify core restriction as starting the instance.
[...]
> --- a/lib/librte_eal/common/eal_common_lcore.c
> +++ b/lib/librte_eal/common/eal_common_lcore.c
> @@ -57,6 +57,12 @@ rte_eal_cpu_init(void)
>  	struct rte_config *config = rte_eal_get_configuration();
>  	unsigned lcore_id;
>  	unsigned count = 0;
> +	rte_cpuset_t affinity_set;
> +	pthread_t tid = pthread_self();
> +

A comment is needed here to explain which errors we are checking.

> +	if (pthread_getaffinity_np(tid, sizeof(rte_cpuset_t),
> +				   &affinity_set) < 0)
> +		CPU_ZERO(&affinity_set);
>  
>  	/*
>  	 * Parse the maximum set of logical cores, detect the subset of running
> @@ -70,7 +76,8 @@ rte_eal_cpu_init(void)
>  
>  		/* in 1:1 mapping, record related cpu detected state */
>  		lcore_config[lcore_id].detected = eal_cpu_detected(lcore_id);
> -		if (lcore_config[lcore_id].detected == 0) {
> +		if (lcore_config[lcore_id].detected == 0 ||
> +		    !CPU_ISSET(lcore_id, &affinity_set)) {
>  			config->lcore_role[lcore_id] = ROLE_OFF;
>  			lcore_config[lcore_id].core_index = -1;
>  			continue;
>
Thomas Monjalon Sept. 16, 2016, 2:04 p.m. | #3
2016-09-02 17:53, Bruce Richardson:
> On Thu, Sep 01, 2016 at 01:31:47AM +0000, Jianfeng Tan wrote:

It would help the discussion to have a problem statement here.

> > This patch uses pthread_getaffinity_np() to narrow down detected
> > cores before parsing coremask (-c), corelist (-l), and coremap
> > (--lcores).
> > 
> > The purpose of this patch is to leave out these core related options
> > when DPDK applications are deployed under container env, so that
> > users only specify core restriction as starting the instance.
> > 
> > Note: previously, some users are using isolated CPUs, which could
> > be excluded by default. Please add commands like taskset to use
> > those cores.
> > 
> > Test example:
> > $ taskset 0xc0000 ./examples/helloworld/build/helloworld -m 1024
> > 
> 
> So, to be clear, does this patch mean that DPDK cannot use isolated cores
> any more unless you explicitly run the app using taskset?
> Is so, NAK, since isolating cores has been part of standard DPDK setup since
> the first versions, and I don't believe that we should break that behaviour.

So how could we help the container use-case?
Any suggestions?

Patch

diff --git a/lib/librte_eal/common/eal_common_lcore.c b/lib/librte_eal/common/eal_common_lcore.c
index 2cd4132..71c575c 100644
--- a/lib/librte_eal/common/eal_common_lcore.c
+++ b/lib/librte_eal/common/eal_common_lcore.c
@@ -57,6 +57,12 @@  rte_eal_cpu_init(void)
 	struct rte_config *config = rte_eal_get_configuration();
 	unsigned lcore_id;
 	unsigned count = 0;
+	rte_cpuset_t affinity_set;
+	pthread_t tid = pthread_self();
+
+	if (pthread_getaffinity_np(tid, sizeof(rte_cpuset_t),
+				   &affinity_set) < 0)
+		CPU_ZERO(&affinity_set);
 
 	/*
 	 * Parse the maximum set of logical cores, detect the subset of running
@@ -70,7 +76,8 @@  rte_eal_cpu_init(void)
 
 		/* in 1:1 mapping, record related cpu detected state */
 		lcore_config[lcore_id].detected = eal_cpu_detected(lcore_id);
-		if (lcore_config[lcore_id].detected == 0) {
+		if (lcore_config[lcore_id].detected == 0 ||
+		    !CPU_ISSET(lcore_id, &affinity_set)) {
 			config->lcore_role[lcore_id] = ROLE_OFF;
 			lcore_config[lcore_id].core_index = -1;
 			continue;