| Message ID | 20181015222110.61564-1-ferruh.yigit@intel.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Thomas Monjalon |
| Headers | show |
| Series | [v2,1/2] eal: add API that sleeps while waiting for threads | expand |
| Context | Check | Description |
|---|---|---|
| ci/Intel-compilation | success | Compilation OK |
| ci/checkpatch | warning | coding style issues |
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
> > +/* > + * 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
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;
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(+)