> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Cunming Liang
> Sent: Thursday, January 22, 2015 9:17 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH v1 15/15] timer: add support to non-EAL thread
> 
> Allow to setup timers only for EAL (lcore) threads (__lcore_id <
> MAX_LCORE_ID).
> E.g. ? dynamically created thread will be able to reset/stop timer for lcore
> thread,
> but it will be not allowed to setup timer for itself or another non-lcore
> thread.
> rte_timer_manage() for non-lcore thread would simply do nothing and
> return straightway.
> 
> Signed-off-by: Cunming Liang <cunming.liang at intel.com>
> ---
>  lib/librte_timer/rte_timer.c | 40 +++++++++++++++++++++++++++++++----
> -----
>  lib/librte_timer/rte_timer.h |  2 +-
>  2 files changed, 32 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c
> index 269a992..601c159 100644
> --- a/lib/librte_timer/rte_timer.c
> +++ b/lib/librte_timer/rte_timer.c
> @@ -79,9 +79,10 @@ static struct priv_timer priv_timer[RTE_MAX_LCORE];
> 

Why not extend the priv_timer size to value being in range returned by 
rte_lcore_id().

All timer stuff will work automatically after such change without any change in 
timer logic including stats.

>  /* when debug is enabled, store some statistics */
>  #ifdef RTE_LIBRTE_TIMER_DEBUG
> -#define __TIMER_STAT_ADD(name, n) do {                               \
> -             unsigned __lcore_id = rte_lcore_id();           \
> -             priv_timer[__lcore_id].stats.name += (n);       \
> +#define __TIMER_STAT_ADD(name, n) do {
>       \
> +             unsigned __lcore_id = rte_lcore_id();                   \
> +             if (__lcore_id < RTE_MAX_LCORE)
>       \
> +                     priv_timer[__lcore_id].stats.name += (n);       \
>       } while(0)
>  #else
>  #define __TIMER_STAT_ADD(name, n) do {} while(0)
> @@ -127,15 +128,26 @@ timer_set_config_state(struct rte_timer *tim,
>       unsigned lcore_id;
> 
>       lcore_id = rte_lcore_id();
> +     if (lcore_id >= RTE_MAX_LCORE)
> +             lcore_id = LCORE_ID_ANY;
> 
>       /* wait that the timer is in correct status before update,
>        * and mark it as being configured */
>       while (success == 0) {
>               prev_status.u32 = tim->status.u32;
> 
> +             /*
> +              * prevent race condition of non-EAL threads
> +              * to update the timer. When 'owner == LCORE_ID_ANY',
> +              * it means updated by a non-EAL thread.
> +              */
> +             if (lcore_id == (unsigned)LCORE_ID_ANY &&
> +                 (uint16_t)lcore_id == prev_status.owner)
> +                     return -1;
> +
>               /* timer is running on another core, exit */
>               if (prev_status.state == RTE_TIMER_RUNNING &&
> -                 (unsigned)prev_status.owner != lcore_id)
> +                 prev_status.owner != (uint16_t)lcore_id)
>                       return -1;
> 
>               /* timer is being configured on another core */
> @@ -366,9 +378,13 @@ __rte_timer_reset(struct rte_timer *tim, uint64_t
> expire,
> 
>       /* round robin for tim_lcore */
>       if (tim_lcore == (unsigned)LCORE_ID_ANY) {
> -             tim_lcore =
> rte_get_next_lcore(priv_timer[lcore_id].prev_lcore,
> -                                            0, 1);
> -             priv_timer[lcore_id].prev_lcore = tim_lcore;
> +             if (lcore_id < RTE_MAX_LCORE) {
> +                     tim_lcore = rte_get_next_lcore(
> +                             priv_timer[lcore_id].prev_lcore,
> +                             0, 1);
> +                     priv_timer[lcore_id].prev_lcore = tim_lcore;
> +             } else
> +                     tim_lcore = rte_get_next_lcore(LCORE_ID_ANY, 0,
> 1);
>       }
> 
>       /* wait that the timer is in correct status before update,
> @@ -378,7 +394,8 @@ __rte_timer_reset(struct rte_timer *tim, uint64_t
> expire,
>               return -1;
> 
>       __TIMER_STAT_ADD(reset, 1);
> -     if (prev_status.state == RTE_TIMER_RUNNING) {
> +     if (prev_status.state == RTE_TIMER_RUNNING &&
> +         lcore_id < RTE_MAX_LCORE) {
>               priv_timer[lcore_id].updated = 1;
>       }
> 
> @@ -455,7 +472,8 @@ rte_timer_stop(struct rte_timer *tim)
>               return -1;
> 
>       __TIMER_STAT_ADD(stop, 1);
> -     if (prev_status.state == RTE_TIMER_RUNNING) {
> +     if (prev_status.state == RTE_TIMER_RUNNING &&
> +         lcore_id < RTE_MAX_LCORE) {
>               priv_timer[lcore_id].updated = 1;
>       }
> 
> @@ -499,6 +517,10 @@ void rte_timer_manage(void)
>       uint64_t cur_time;
>       int i, ret;
> 
> +     /* timer manager only runs on EAL thread */
> +     if (lcore_id >= RTE_MAX_LCORE)
> +             return;
> +
>       __TIMER_STAT_ADD(manage, 1);
>       /* optimize for the case where per-cpu list is empty */
>       if (priv_timer[lcore_id].pending_head.sl_next[0] == NULL)
> diff --git a/lib/librte_timer/rte_timer.h b/lib/librte_timer/rte_timer.h
> index 4907cf5..5c5df91 100644
> --- a/lib/librte_timer/rte_timer.h
> +++ b/lib/librte_timer/rte_timer.h
> @@ -76,7 +76,7 @@ extern "C" {
>  #define RTE_TIMER_RUNNING 2 /**< State: timer function is running. */
>  #define RTE_TIMER_CONFIG  3 /**< State: timer is being configured. */
> 
> -#define RTE_TIMER_NO_OWNER -1 /**< Timer has no owner. */
> +#define RTE_TIMER_NO_OWNER -2 /**< Timer has no owner. */
> 
>  /**
>   * Timer type: Periodic or single (one-shot).
> --
> 1.8.1.4

Reply via email to