[v2,1/2] eal: add API that sleeps while waiting for threads
diff mbox series

Message ID 20181015222110.61564-1-ferruh.yigit@intel.com
State New
Delegated to: Thomas Monjalon
Headers show
Series
  • [v2,1/2] eal: add API that sleeps while waiting for threads
Related show

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/checkpatch warning coding style issues

Commit Message

Ferruh Yigit Oct. 15, 2018, 10:21 p.m. UTC
It is common that sample applications call rte_eal_wait_lcore() while
waiting for worker threads to be terminated.
Mostly master lcore keeps waiting in this function.

The waiting app for termination is not a time critical task, app can
prefer a sleep version of the waiting to consume less cycles.

A sleeping version of the API, rte_eal_wait_lcore_sleep(), has been
added which uses pthread conditions.

Sample applications will be updated later to use this API.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
v2:
* use pthread cond instead of usleep
---
 lib/librte_eal/bsdapp/eal/eal.c            |  3 +++
 lib/librte_eal/bsdapp/eal/eal_thread.c     |  7 ++++++
 lib/librte_eal/common/eal_common_launch.c  | 22 ++++++++++++++++++
 lib/librte_eal/common/include/rte_launch.h | 26 ++++++++++++++++++++++
 lib/librte_eal/common/include/rte_lcore.h  |  3 +++
 lib/librte_eal/linuxapp/eal/eal.c          |  3 +++
 lib/librte_eal/linuxapp/eal/eal_thread.c   |  7 ++++++
 lib/librte_eal/rte_eal_version.map         |  1 +
 8 files changed, 72 insertions(+)

Comments

Ananyev, Konstantin Oct. 16, 2018, 8:42 a.m. UTC | #1
HI Ferruh,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
> Sent: Monday, October 15, 2018 11:21 PM
> To: Richardson, Bruce <bruce.richardson@intel.com>
> Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>; stephen@networkplumber.org
> Subject: [dpdk-dev] [PATCH v2 1/2] eal: add API that sleeps while waiting for threads
> 
> It is common that sample applications call rte_eal_wait_lcore() while
> waiting for worker threads to be terminated.
> Mostly master lcore keeps waiting in this function.
> 
> The waiting app for termination is not a time critical task, app can
> prefer a sleep version of the waiting to consume less cycles.
> 
> A sleeping version of the API, rte_eal_wait_lcore_sleep(), has been
> added which uses pthread conditions.
> 
> Sample applications will be updated later to use this API.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> v2:
> * use pthread cond instead of usleep
> ---
>  lib/librte_eal/bsdapp/eal/eal.c            |  3 +++
>  lib/librte_eal/bsdapp/eal/eal_thread.c     |  7 ++++++
>  lib/librte_eal/common/eal_common_launch.c  | 22 ++++++++++++++++++
>  lib/librte_eal/common/include/rte_launch.h | 26 ++++++++++++++++++++++
>  lib/librte_eal/common/include/rte_lcore.h  |  3 +++
>  lib/librte_eal/linuxapp/eal/eal.c          |  3 +++
>  lib/librte_eal/linuxapp/eal/eal_thread.c   |  7 ++++++
>  lib/librte_eal/rte_eal_version.map         |  1 +
>  8 files changed, 72 insertions(+)
> 
> diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
> index 7735194a3..e7d676657 100644
> --- a/lib/librte_eal/bsdapp/eal/eal.c
> +++ b/lib/librte_eal/bsdapp/eal/eal.c
> @@ -756,6 +756,9 @@ rte_eal_init(int argc, char **argv)
>  		snprintf(thread_name, sizeof(thread_name),
>  				"lcore-slave-%d", i);
>  		rte_thread_setname(lcore_config[i].thread_id, thread_name);
> +
> +		pthread_mutex_init(&rte_eal_thread_mutex[i], NULL);
> +		pthread_cond_init(&rte_eal_thread_cond[i], NULL);
>  	}
> 
>  	/*
> diff --git a/lib/librte_eal/bsdapp/eal/eal_thread.c b/lib/librte_eal/bsdapp/eal/eal_thread.c
> index 309b58726..60db32d57 100644
> --- a/lib/librte_eal/bsdapp/eal/eal_thread.c
> +++ b/lib/librte_eal/bsdapp/eal/eal_thread.c
> @@ -28,6 +28,9 @@ RTE_DEFINE_PER_LCORE(unsigned, _lcore_id) = LCORE_ID_ANY;
>  RTE_DEFINE_PER_LCORE(unsigned, _socket_id) = (unsigned)SOCKET_ID_ANY;
>  RTE_DEFINE_PER_LCORE(rte_cpuset_t, _cpuset);
> 
> +pthread_cond_t rte_eal_thread_cond[RTE_MAX_LCORE];
> +pthread_mutex_t rte_eal_thread_mutex[RTE_MAX_LCORE];

I think would be better to include cond and mutex into struct lcore_config itself,
probably would help to avoid false sharing.
Though yeh, it would mean ABI breakage, I suppose. 

> +
>  /*
>   * Send a message to a slave lcore identified by slave_id to call a
>   * function f with argument arg. Once the execution is done, the
> @@ -154,6 +157,10 @@ eal_thread_loop(__attribute__((unused)) void *arg)
>  		lcore_config[lcore_id].ret = ret;
>  		rte_wmb();
>  		lcore_config[lcore_id].state = FINISHED;
> +
> +		pthread_mutex_lock(&rte_eal_thread_mutex[lcore_id]);
> +		pthread_cond_signal(&rte_eal_thread_cond[lcore_id]);
> +		pthread_mutex_unlock(&rte_eal_thread_mutex[lcore_id]);

I understand it would work that way too, but if you introduce mutex and cond around  
the state, then it is better to manipulate/access the state after grabbing the mutex.
BTW in that case we don't need wmb:

lcore_config[lcore_id].ret = ret;
pthread_mutex_lock(...);
lcore_config[lcore_id].state = FINISHED;
pthread_cond_signal(..);
pthread_mutex_unlock(...);

Konstantin
Ananyev, Konstantin Oct. 16, 2018, 9:05 a.m. UTC | #2
> 
> +/*
> + * Wait until a lcore finished its job by pthread condition.
> + */
> +int
> +rte_eal_wait_lcore_sleep(unsigned slave_id)
> +{
> +	if (lcore_config[slave_id].state == WAIT)
> +		return 0;
> +
> +	pthread_mutex_lock(&rte_eal_thread_mutex[slave_id]);
> +	while (lcore_config[slave_id].state != WAIT &&
> +	       lcore_config[slave_id].state != FINISHED)
> +		pthread_cond_wait(&rte_eal_thread_cond[slave_id],
> +				&rte_eal_thread_mutex[slave_id]);
> +	pthread_mutex_unlock(&rte_eal_thread_mutex[slave_id]);
> +
> +	/* we are in finished state, go to wait state */
> +	lcore_config[slave_id].state = WAIT;
> +	return lcore_config[slave_id].ret;
> +}
> +

Actually, another question - could that 2 or more threads wait for the same core simultaneously?
If yes, then 2-nd thread in that function might stuck forever.
In that case it is better to use cond_timed_wait() here and cond_broadcast() above.
Konstantin

Patch
diff mbox series

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 7735194a3..e7d676657 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -756,6 +756,9 @@  rte_eal_init(int argc, char **argv)
 		snprintf(thread_name, sizeof(thread_name),
 				"lcore-slave-%d", i);
 		rte_thread_setname(lcore_config[i].thread_id, thread_name);
+
+		pthread_mutex_init(&rte_eal_thread_mutex[i], NULL);
+		pthread_cond_init(&rte_eal_thread_cond[i], NULL);
 	}
 
 	/*
diff --git a/lib/librte_eal/bsdapp/eal/eal_thread.c b/lib/librte_eal/bsdapp/eal/eal_thread.c
index 309b58726..60db32d57 100644
--- a/lib/librte_eal/bsdapp/eal/eal_thread.c
+++ b/lib/librte_eal/bsdapp/eal/eal_thread.c
@@ -28,6 +28,9 @@  RTE_DEFINE_PER_LCORE(unsigned, _lcore_id) = LCORE_ID_ANY;
 RTE_DEFINE_PER_LCORE(unsigned, _socket_id) = (unsigned)SOCKET_ID_ANY;
 RTE_DEFINE_PER_LCORE(rte_cpuset_t, _cpuset);
 
+pthread_cond_t rte_eal_thread_cond[RTE_MAX_LCORE];
+pthread_mutex_t rte_eal_thread_mutex[RTE_MAX_LCORE];
+
 /*
  * Send a message to a slave lcore identified by slave_id to call a
  * function f with argument arg. Once the execution is done, the
@@ -154,6 +157,10 @@  eal_thread_loop(__attribute__((unused)) void *arg)
 		lcore_config[lcore_id].ret = ret;
 		rte_wmb();
 		lcore_config[lcore_id].state = FINISHED;
+
+		pthread_mutex_lock(&rte_eal_thread_mutex[lcore_id]);
+		pthread_cond_signal(&rte_eal_thread_cond[lcore_id]);
+		pthread_mutex_unlock(&rte_eal_thread_mutex[lcore_id]);
 	}
 
 	/* never reached */
diff --git a/lib/librte_eal/common/eal_common_launch.c b/lib/librte_eal/common/eal_common_launch.c
index fe0ba3f0d..ebfa65847 100644
--- a/lib/librte_eal/common/eal_common_launch.c
+++ b/lib/librte_eal/common/eal_common_launch.c
@@ -5,6 +5,7 @@ 
 #include <errno.h>
 #include <stdint.h>
 #include <stdio.h>
+#include <unistd.h>
 #include <sys/queue.h>
 
 #include <rte_launch.h>
@@ -35,6 +36,27 @@  rte_eal_wait_lcore(unsigned slave_id)
 	return lcore_config[slave_id].ret;
 }
 
+/*
+ * Wait until a lcore finished its job by pthread condition.
+ */
+int
+rte_eal_wait_lcore_sleep(unsigned slave_id)
+{
+	if (lcore_config[slave_id].state == WAIT)
+		return 0;
+
+	pthread_mutex_lock(&rte_eal_thread_mutex[slave_id]);
+	while (lcore_config[slave_id].state != WAIT &&
+	       lcore_config[slave_id].state != FINISHED)
+		pthread_cond_wait(&rte_eal_thread_cond[slave_id],
+				&rte_eal_thread_mutex[slave_id]);
+	pthread_mutex_unlock(&rte_eal_thread_mutex[slave_id]);
+
+	/* we are in finished state, go to wait state */
+	lcore_config[slave_id].state = WAIT;
+	return lcore_config[slave_id].ret;
+}
+
 /*
  * Check that every SLAVE lcores are in WAIT state, then call
  * rte_eal_remote_launch() for all of them. If call_master is true
diff --git a/lib/librte_eal/common/include/rte_launch.h b/lib/librte_eal/common/include/rte_launch.h
index 06a671752..0306f7c3a 100644
--- a/lib/librte_eal/common/include/rte_launch.h
+++ b/lib/librte_eal/common/include/rte_launch.h
@@ -11,6 +11,8 @@ 
  * Launch tasks on other lcores
  */
 
+#include <rte_compat.h>
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -129,6 +131,30 @@  enum rte_lcore_state_t rte_eal_get_lcore_state(unsigned slave_id);
  */
 int rte_eal_wait_lcore(unsigned slave_id);
 
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Wait until an lcore finishes its job.
+ *
+ * To be executed on the MASTER lcore only.
+ *
+ * Same as rte_eal_wait_lcore() but waits using pthread conditions
+ * instead of polling in busy loop.
+ *
+ * @param slave_id
+ *   The identifier of the lcore.
+ * @return
+ *   - 0: If the lcore identified by the slave_id is in a WAIT state.
+ *   - The value that was returned by the previous remote launch
+ *     function call if the lcore identified by the slave_id was in a
+ *     FINISHED or RUNNING state. In this case, it changes the state
+ *     of the lcore to WAIT.
+ */
+__rte_experimental int
+rte_eal_wait_lcore_sleep(unsigned slave_id);
+
 /**
  * Wait until all lcores finish their jobs.
  *
diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h
index 6e09d9181..9ce8bf643 100644
--- a/lib/librte_eal/common/include/rte_lcore.h
+++ b/lib/librte_eal/common/include/rte_lcore.h
@@ -53,6 +53,9 @@  struct lcore_config {
  */
 extern struct lcore_config lcore_config[RTE_MAX_LCORE];
 
+extern pthread_cond_t rte_eal_thread_cond[RTE_MAX_LCORE];
+extern pthread_mutex_t rte_eal_thread_mutex[RTE_MAX_LCORE];
+
 RTE_DECLARE_PER_LCORE(unsigned, _lcore_id);  /**< Per thread "lcore id". */
 RTE_DECLARE_PER_LCORE(rte_cpuset_t, _cpuset); /**< Per thread "cpuset". */
 
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 950f33f2c..9d69a0642 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -1019,6 +1019,9 @@  rte_eal_init(int argc, char **argv)
 
 		lcore_config[i].state = WAIT;
 
+		pthread_mutex_init(&rte_eal_thread_mutex[i], NULL);
+		pthread_cond_init(&rte_eal_thread_cond[i], NULL);
+
 		/* create a thread for each lcore */
 		ret = pthread_create(&lcore_config[i].thread_id, NULL,
 				     eal_thread_loop, NULL);
diff --git a/lib/librte_eal/linuxapp/eal/eal_thread.c b/lib/librte_eal/linuxapp/eal/eal_thread.c
index b496fc711..5381c7aa2 100644
--- a/lib/librte_eal/linuxapp/eal/eal_thread.c
+++ b/lib/librte_eal/linuxapp/eal/eal_thread.c
@@ -28,6 +28,9 @@  RTE_DEFINE_PER_LCORE(unsigned, _lcore_id) = LCORE_ID_ANY;
 RTE_DEFINE_PER_LCORE(unsigned, _socket_id) = (unsigned)SOCKET_ID_ANY;
 RTE_DEFINE_PER_LCORE(rte_cpuset_t, _cpuset);
 
+pthread_cond_t rte_eal_thread_cond[RTE_MAX_LCORE];
+pthread_mutex_t rte_eal_thread_mutex[RTE_MAX_LCORE];
+
 /*
  * Send a message to a slave lcore identified by slave_id to call a
  * function f with argument arg. Once the execution is done, the
@@ -161,6 +164,10 @@  eal_thread_loop(__attribute__((unused)) void *arg)
 			lcore_config[lcore_id].state = WAIT;
 		else
 			lcore_config[lcore_id].state = FINISHED;
+
+		pthread_mutex_lock(&rte_eal_thread_mutex[lcore_id]);
+		pthread_cond_signal(&rte_eal_thread_cond[lcore_id]);
+		pthread_mutex_unlock(&rte_eal_thread_mutex[lcore_id]);
 	}
 
 	/* never reached */
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index e968edc2e..6c636a65d 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -292,6 +292,7 @@  EXPERIMENTAL {
 	rte_devargs_remove;
 	rte_devargs_type_count;
 	rte_eal_cleanup;
+	rte_eal_wait_lcore_sleep;
 	rte_fbarray_attach;
 	rte_fbarray_destroy;
 	rte_fbarray_detach;