[dpdk-dev,RFC] timer: inform periodic timers of multiple expiries

Message ID 20170428132538.15995-1-bruce.richardson@intel.com (mailing list archive)
State RFC, archived
Headers

Checks

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

Commit Message

Bruce Richardson April 28, 2017, 1:25 p.m. UTC
  if timer_manage is called much less frequently than the period of a
periodic timer, then timer expiries will be missed. For example, if a timer
has a period of 300us, but timer_manage is called every 1ms, then there
will only be one timer callback called every 1ms instead of 3 within that
time.

While we can fix this by having each function called multiple times within
timer-manage, this will lead to out-of-order timeouts, and will be slower
with all the function call overheads - especially in the case of a timeout
doing something trivial like incrementing a counter. Therefore, we instead
modify the callback functions to take a counter value of the number of
expiries that have passed since the last time it was called.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 examples/l2fwd-jobstats/main.c                     |  7 +++++--
 examples/l2fwd-keepalive/main.c                    |  8 ++++----
 examples/l3fwd-power/main.c                        |  5 +++--
 examples/performance-thread/common/lthread_sched.c |  4 +++-
 examples/performance-thread/common/lthread_sched.h |  2 +-
 examples/timer/main.c                              | 10 ++++++----
 lib/librte_timer/rte_timer.c                       |  9 ++++++++-
 lib/librte_timer/rte_timer.h                       |  2 +-
 test/test/test_timer.c                             | 14 +++++++++-----
 test/test/test_timer_perf.c                        |  4 +++-
 test/test/test_timer_racecond.c                    |  3 ++-
 11 files changed, 45 insertions(+), 23 deletions(-)
  

Comments

Bruce Richardson April 28, 2017, 1:29 p.m. UTC | #1
I'd like some agreement soon on the approach to be taken to fix this issue,
in case we need an ABI change notice in 17.05 - i.e. if we take the
approach given in the patch below.

Also, while the alternative solution of calling a function multiple
times is not an ABI/API change, I view it as more problematic from a
compatibility point of view, as it would be a "silent" functionality
change for existing apps.

Regards,
/Bruce

On Fri, Apr 28, 2017 at 02:25:38PM +0100, Bruce Richardson wrote:
> if timer_manage is called much less frequently than the period of a
> periodic timer, then timer expiries will be missed. For example, if a timer
> has a period of 300us, but timer_manage is called every 1ms, then there
> will only be one timer callback called every 1ms instead of 3 within that
> time.
> 
> While we can fix this by having each function called multiple times within
> timer-manage, this will lead to out-of-order timeouts, and will be slower
> with all the function call overheads - especially in the case of a timeout
> doing something trivial like incrementing a counter. Therefore, we instead
> modify the callback functions to take a counter value of the number of
> expiries that have passed since the last time it was called.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  examples/l2fwd-jobstats/main.c                     |  7 +++++--
>  examples/l2fwd-keepalive/main.c                    |  8 ++++----
>  examples/l3fwd-power/main.c                        |  5 +++--
>  examples/performance-thread/common/lthread_sched.c |  4 +++-
>  examples/performance-thread/common/lthread_sched.h |  2 +-
>  examples/timer/main.c                              | 10 ++++++----
>  lib/librte_timer/rte_timer.c                       |  9 ++++++++-
>  lib/librte_timer/rte_timer.h                       |  2 +-
>  test/test/test_timer.c                             | 14 +++++++++-----
>  test/test/test_timer_perf.c                        |  4 +++-
>  test/test/test_timer_racecond.c                    |  3 ++-
>  11 files changed, 45 insertions(+), 23 deletions(-)
> 
> diff --git a/examples/l2fwd-jobstats/main.c b/examples/l2fwd-jobstats/main.c
> index e6e6c22..b264344 100644
> --- a/examples/l2fwd-jobstats/main.c
> +++ b/examples/l2fwd-jobstats/main.c
> @@ -410,7 +410,8 @@ l2fwd_job_update_cb(struct rte_jobstats *job, int64_t result)
>  }
>  
>  static void
> -l2fwd_fwd_job(__rte_unused struct rte_timer *timer, void *arg)
> +l2fwd_fwd_job(__rte_unused struct rte_timer *timer,
> +		__rte_unused unsigned int count, void *arg)
>  {
>  	struct rte_mbuf *pkts_burst[MAX_PKT_BURST];
>  	struct rte_mbuf *m;
> @@ -460,7 +461,9 @@ l2fwd_fwd_job(__rte_unused struct rte_timer *timer, void *arg)
>  }
>  
>  static void
> -l2fwd_flush_job(__rte_unused struct rte_timer *timer, __rte_unused void *arg)
> +l2fwd_flush_job(__rte_unused struct rte_timer *timer,
> +		__rte_unused unsigned int count,
> +		__rte_unused void *arg)
>  {
>  	uint64_t now;
>  	unsigned lcore_id;
> diff --git a/examples/l2fwd-keepalive/main.c b/examples/l2fwd-keepalive/main.c
> index 4623d2a..26eba12 100644
> --- a/examples/l2fwd-keepalive/main.c
> +++ b/examples/l2fwd-keepalive/main.c
> @@ -144,8 +144,9 @@ struct rte_keepalive *rte_global_keepalive_info;
>  
>  /* Print out statistics on packets dropped */
>  static void
> -print_stats(__attribute__((unused)) struct rte_timer *ptr_timer,
> -	__attribute__((unused)) void *ptr_data)
> +print_stats(__rte_unused struct rte_timer *ptr_timer,
> +		__rte_unused unsigned int count,
> +		__rte_unused void *ptr_data)
>  {
>  	uint64_t total_packets_dropped, total_packets_tx, total_packets_rx;
>  	unsigned portid;
> @@ -748,8 +749,7 @@ main(int argc, char **argv)
>  				(check_period * rte_get_timer_hz()) / 1000,
>  				PERIODICAL,
>  				rte_lcore_id(),
> -				(void(*)(struct rte_timer*, void*))
> -				&rte_keepalive_dispatch_pings,
> +				(void *)&rte_keepalive_dispatch_pings,
>  				rte_global_keepalive_info
>  				) != 0 )
>  			rte_exit(EXIT_FAILURE, "Keepalive setup failure.\n");
> diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
> index ec40a17..318aefd 100644
> --- a/examples/l3fwd-power/main.c
> +++ b/examples/l3fwd-power/main.c
> @@ -410,8 +410,9 @@ signal_exit_now(int sigtype)
>  
>  /*  Freqency scale down timer callback */
>  static void
> -power_timer_cb(__attribute__((unused)) struct rte_timer *tim,
> -			  __attribute__((unused)) void *arg)
> +power_timer_cb(__rte_unused struct rte_timer *tim,
> +		__rte_unused unsigned int count,
> +		__rte_unused void *arg)
>  {
>  	uint64_t hz;
>  	float sleep_time_ratio;
> diff --git a/examples/performance-thread/common/lthread_sched.c b/examples/performance-thread/common/lthread_sched.c
> index c64c21f..c4c7d3b 100644
> --- a/examples/performance-thread/common/lthread_sched.c
> +++ b/examples/performance-thread/common/lthread_sched.c
> @@ -437,7 +437,9 @@ static inline void _lthread_resume(struct lthread *lt)
>   * Handle sleep timer expiry
>  */
>  void
> -_sched_timer_cb(struct rte_timer *tim, void *arg)
> +_sched_timer_cb(struct rte_timer *tim,
> +		unsigned int count __rte_unused,
> +		void *arg)
>  {
>  	struct lthread *lt = (struct lthread *) arg;
>  	uint64_t state = lt->state;
> diff --git a/examples/performance-thread/common/lthread_sched.h b/examples/performance-thread/common/lthread_sched.h
> index 7cddda9..f2af8b3 100644
> --- a/examples/performance-thread/common/lthread_sched.h
> +++ b/examples/performance-thread/common/lthread_sched.h
> @@ -149,7 +149,7 @@ _reschedule(void)
>  }
>  
>  extern struct lthread_sched *schedcore[];
> -void _sched_timer_cb(struct rte_timer *tim, void *arg);
> +void _sched_timer_cb(struct rte_timer *tim, unsigned int count, void *arg);
>  void _sched_shutdown(__rte_unused void *arg);
>  
>  #ifdef __cplusplus
> diff --git a/examples/timer/main.c b/examples/timer/main.c
> index 37ad559..92a6a1f 100644
> --- a/examples/timer/main.c
> +++ b/examples/timer/main.c
> @@ -55,8 +55,9 @@ static struct rte_timer timer1;
>  
>  /* timer0 callback */
>  static void
> -timer0_cb(__attribute__((unused)) struct rte_timer *tim,
> -	  __attribute__((unused)) void *arg)
> +timer0_cb(__rte_unused struct rte_timer *tim,
> +	  __rte_unused unsigned int count,
> +	  __rte_unused void *arg)
>  {
>  	static unsigned counter = 0;
>  	unsigned lcore_id = rte_lcore_id();
> @@ -71,8 +72,9 @@ timer0_cb(__attribute__((unused)) struct rte_timer *tim,
>  
>  /* timer1 callback */
>  static void
> -timer1_cb(__attribute__((unused)) struct rte_timer *tim,
> -	  __attribute__((unused)) void *arg)
> +timer1_cb(__rte_unused struct rte_timer *tim,
> +	  __rte_unused unsigned int count,
> +	  __rte_unused void *arg)
>  {
>  	unsigned lcore_id = rte_lcore_id();
>  	uint64_t hz;
> diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c
> index 18782fa..d1e2c12 100644
> --- a/lib/librte_timer/rte_timer.c
> +++ b/lib/librte_timer/rte_timer.c
> @@ -590,7 +590,14 @@ void rte_timer_manage(void)
>  		priv_timer[lcore_id].running_tim = tim;
>  
>  		/* execute callback function with list unlocked */
> -		tim->f(tim, tim->arg);
> +		if (tim->period == 0)
> +			tim->f(tim, 1, tim->arg);
> +		else {
> +			/* for periodic check how many expiries we have */
> +			uint64_t over_time = cur_time - tim->expire;
> +			unsigned int extra_expiries = over_time / tim->period;
> +			tim->f(tim, 1 + extra_expiries, tim->arg);
> +		}
>  
>  		__TIMER_STAT_ADD(pending, -1);
>  		/* the timer was stopped or reloaded by the callback
> diff --git a/lib/librte_timer/rte_timer.h b/lib/librte_timer/rte_timer.h
> index a276a73..bc434ec 100644
> --- a/lib/librte_timer/rte_timer.h
> +++ b/lib/librte_timer/rte_timer.h
> @@ -117,7 +117,7 @@ struct rte_timer;
>  /**
>   * Callback function type for timer expiry.
>   */
> -typedef void (*rte_timer_cb_t)(struct rte_timer *, void *);
> +typedef void (*rte_timer_cb_t)(struct rte_timer *, unsigned int count, void *);
>  
>  #define MAX_SKIPLIST_DEPTH 10
>  
> diff --git a/test/test/test_timer.c b/test/test/test_timer.c
> index 2f6525a..0b86d3c 100644
> --- a/test/test/test_timer.c
> +++ b/test/test/test_timer.c
> @@ -153,7 +153,8 @@ struct mytimerinfo {
>  
>  static struct mytimerinfo mytiminfo[NB_TIMER];
>  
> -static void timer_basic_cb(struct rte_timer *tim, void *arg);
> +static void
> +timer_basic_cb(struct rte_timer *tim, unsigned int count, void *arg);
>  
>  static void
>  mytimer_reset(struct mytimerinfo *timinfo, uint64_t ticks,
> @@ -167,6 +168,7 @@ mytimer_reset(struct mytimerinfo *timinfo, uint64_t ticks,
>  /* timer callback for stress tests */
>  static void
>  timer_stress_cb(__attribute__((unused)) struct rte_timer *tim,
> +		__attribute__((unused)) unsigned int count,
>  		__attribute__((unused)) void *arg)
>  {
>  	long r;
> @@ -293,9 +295,11 @@ static volatile int cb_count = 0;
>  /* callback for second stress test. will only be called
>   * on master lcore */
>  static void
> -timer_stress2_cb(struct rte_timer *tim __rte_unused, void *arg __rte_unused)
> +timer_stress2_cb(struct rte_timer *tim __rte_unused,
> +		unsigned int count,
> +		void *arg __rte_unused)
>  {
> -	cb_count++;
> +	cb_count += count;
>  }
>  
>  #define NB_STRESS2_TIMERS 8192
> @@ -430,7 +434,7 @@ timer_stress2_main_loop(__attribute__((unused)) void *arg)
>  
>  /* timer callback for basic tests */
>  static void
> -timer_basic_cb(struct rte_timer *tim, void *arg)
> +timer_basic_cb(struct rte_timer *tim, unsigned int count, void *arg)
>  {
>  	struct mytimerinfo *timinfo = arg;
>  	uint64_t hz = rte_get_timer_hz();
> @@ -440,7 +444,7 @@ timer_basic_cb(struct rte_timer *tim, void *arg)
>  	if (rte_timer_pending(tim))
>  		return;
>  
> -	timinfo->count ++;
> +	timinfo->count += count;
>  
>  	RTE_LOG(INFO, TESTTIMER,
>  		"%"PRIu64": callback id=%u count=%u on core %u\n",
> diff --git a/test/test/test_timer_perf.c b/test/test/test_timer_perf.c
> index fa77efb..5b3867d 100644
> --- a/test/test/test_timer_perf.c
> +++ b/test/test/test_timer_perf.c
> @@ -48,7 +48,9 @@
>  int outstanding_count = 0;
>  
>  static void
> -timer_cb(struct rte_timer *t __rte_unused, void *param __rte_unused)
> +timer_cb(struct rte_timer *t __rte_unused,
> +		unsigned int count __rte_unused,
> +		void *param __rte_unused)
>  {
>  	outstanding_count--;
>  }
> diff --git a/test/test/test_timer_racecond.c b/test/test/test_timer_racecond.c
> index 7824ec4..b1c5c86 100644
> --- a/test/test/test_timer_racecond.c
> +++ b/test/test/test_timer_racecond.c
> @@ -65,7 +65,8 @@ static volatile unsigned stop_slaves;
>  static int reload_timer(struct rte_timer *tim);
>  
>  static void
> -timer_cb(struct rte_timer *tim, void *arg __rte_unused)
> +timer_cb(struct rte_timer *tim, unsigned int count __rte_unused,
> +		void *arg __rte_unused)
>  {
>  	/* Simulate slow callback function, 100 us. */
>  	rte_delay_us(100);
> -- 
> 2.9.3
>
  
Bruce Richardson April 28, 2017, 1:30 p.m. UTC | #2
+maintainer

On Fri, Apr 28, 2017 at 02:25:38PM +0100, Bruce Richardson wrote:
> if timer_manage is called much less frequently than the period of a
> periodic timer, then timer expiries will be missed. For example, if a timer
> has a period of 300us, but timer_manage is called every 1ms, then there
> will only be one timer callback called every 1ms instead of 3 within that
> time.
> 
> While we can fix this by having each function called multiple times within
> timer-manage, this will lead to out-of-order timeouts, and will be slower
> with all the function call overheads - especially in the case of a timeout
> doing something trivial like incrementing a counter. Therefore, we instead
> modify the callback functions to take a counter value of the number of
> expiries that have passed since the last time it was called.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  examples/l2fwd-jobstats/main.c                     |  7 +++++--
>  examples/l2fwd-keepalive/main.c                    |  8 ++++----
>  examples/l3fwd-power/main.c                        |  5 +++--
>  examples/performance-thread/common/lthread_sched.c |  4 +++-
>  examples/performance-thread/common/lthread_sched.h |  2 +-
>  examples/timer/main.c                              | 10 ++++++----
>  lib/librte_timer/rte_timer.c                       |  9 ++++++++-
>  lib/librte_timer/rte_timer.h                       |  2 +-
>  test/test/test_timer.c                             | 14 +++++++++-----
>  test/test/test_timer_perf.c                        |  4 +++-
>  test/test/test_timer_racecond.c                    |  3 ++-
>  11 files changed, 45 insertions(+), 23 deletions(-)
> 
> diff --git a/examples/l2fwd-jobstats/main.c b/examples/l2fwd-jobstats/main.c
> index e6e6c22..b264344 100644
> --- a/examples/l2fwd-jobstats/main.c
> +++ b/examples/l2fwd-jobstats/main.c
> @@ -410,7 +410,8 @@ l2fwd_job_update_cb(struct rte_jobstats *job, int64_t result)
>  }
>  
>  static void
> -l2fwd_fwd_job(__rte_unused struct rte_timer *timer, void *arg)
> +l2fwd_fwd_job(__rte_unused struct rte_timer *timer,
> +		__rte_unused unsigned int count, void *arg)
>  {
>  	struct rte_mbuf *pkts_burst[MAX_PKT_BURST];
>  	struct rte_mbuf *m;
> @@ -460,7 +461,9 @@ l2fwd_fwd_job(__rte_unused struct rte_timer *timer, void *arg)
>  }
>  
>  static void
> -l2fwd_flush_job(__rte_unused struct rte_timer *timer, __rte_unused void *arg)
> +l2fwd_flush_job(__rte_unused struct rte_timer *timer,
> +		__rte_unused unsigned int count,
> +		__rte_unused void *arg)
>  {
>  	uint64_t now;
>  	unsigned lcore_id;
> diff --git a/examples/l2fwd-keepalive/main.c b/examples/l2fwd-keepalive/main.c
> index 4623d2a..26eba12 100644
> --- a/examples/l2fwd-keepalive/main.c
> +++ b/examples/l2fwd-keepalive/main.c
> @@ -144,8 +144,9 @@ struct rte_keepalive *rte_global_keepalive_info;
>  
>  /* Print out statistics on packets dropped */
>  static void
> -print_stats(__attribute__((unused)) struct rte_timer *ptr_timer,
> -	__attribute__((unused)) void *ptr_data)
> +print_stats(__rte_unused struct rte_timer *ptr_timer,
> +		__rte_unused unsigned int count,
> +		__rte_unused void *ptr_data)
>  {
>  	uint64_t total_packets_dropped, total_packets_tx, total_packets_rx;
>  	unsigned portid;
> @@ -748,8 +749,7 @@ main(int argc, char **argv)
>  				(check_period * rte_get_timer_hz()) / 1000,
>  				PERIODICAL,
>  				rte_lcore_id(),
> -				(void(*)(struct rte_timer*, void*))
> -				&rte_keepalive_dispatch_pings,
> +				(void *)&rte_keepalive_dispatch_pings,
>  				rte_global_keepalive_info
>  				) != 0 )
>  			rte_exit(EXIT_FAILURE, "Keepalive setup failure.\n");
> diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
> index ec40a17..318aefd 100644
> --- a/examples/l3fwd-power/main.c
> +++ b/examples/l3fwd-power/main.c
> @@ -410,8 +410,9 @@ signal_exit_now(int sigtype)
>  
>  /*  Freqency scale down timer callback */
>  static void
> -power_timer_cb(__attribute__((unused)) struct rte_timer *tim,
> -			  __attribute__((unused)) void *arg)
> +power_timer_cb(__rte_unused struct rte_timer *tim,
> +		__rte_unused unsigned int count,
> +		__rte_unused void *arg)
>  {
>  	uint64_t hz;
>  	float sleep_time_ratio;
> diff --git a/examples/performance-thread/common/lthread_sched.c b/examples/performance-thread/common/lthread_sched.c
> index c64c21f..c4c7d3b 100644
> --- a/examples/performance-thread/common/lthread_sched.c
> +++ b/examples/performance-thread/common/lthread_sched.c
> @@ -437,7 +437,9 @@ static inline void _lthread_resume(struct lthread *lt)
>   * Handle sleep timer expiry
>  */
>  void
> -_sched_timer_cb(struct rte_timer *tim, void *arg)
> +_sched_timer_cb(struct rte_timer *tim,
> +		unsigned int count __rte_unused,
> +		void *arg)
>  {
>  	struct lthread *lt = (struct lthread *) arg;
>  	uint64_t state = lt->state;
> diff --git a/examples/performance-thread/common/lthread_sched.h b/examples/performance-thread/common/lthread_sched.h
> index 7cddda9..f2af8b3 100644
> --- a/examples/performance-thread/common/lthread_sched.h
> +++ b/examples/performance-thread/common/lthread_sched.h
> @@ -149,7 +149,7 @@ _reschedule(void)
>  }
>  
>  extern struct lthread_sched *schedcore[];
> -void _sched_timer_cb(struct rte_timer *tim, void *arg);
> +void _sched_timer_cb(struct rte_timer *tim, unsigned int count, void *arg);
>  void _sched_shutdown(__rte_unused void *arg);
>  
>  #ifdef __cplusplus
> diff --git a/examples/timer/main.c b/examples/timer/main.c
> index 37ad559..92a6a1f 100644
> --- a/examples/timer/main.c
> +++ b/examples/timer/main.c
> @@ -55,8 +55,9 @@ static struct rte_timer timer1;
>  
>  /* timer0 callback */
>  static void
> -timer0_cb(__attribute__((unused)) struct rte_timer *tim,
> -	  __attribute__((unused)) void *arg)
> +timer0_cb(__rte_unused struct rte_timer *tim,
> +	  __rte_unused unsigned int count,
> +	  __rte_unused void *arg)
>  {
>  	static unsigned counter = 0;
>  	unsigned lcore_id = rte_lcore_id();
> @@ -71,8 +72,9 @@ timer0_cb(__attribute__((unused)) struct rte_timer *tim,
>  
>  /* timer1 callback */
>  static void
> -timer1_cb(__attribute__((unused)) struct rte_timer *tim,
> -	  __attribute__((unused)) void *arg)
> +timer1_cb(__rte_unused struct rte_timer *tim,
> +	  __rte_unused unsigned int count,
> +	  __rte_unused void *arg)
>  {
>  	unsigned lcore_id = rte_lcore_id();
>  	uint64_t hz;
> diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c
> index 18782fa..d1e2c12 100644
> --- a/lib/librte_timer/rte_timer.c
> +++ b/lib/librte_timer/rte_timer.c
> @@ -590,7 +590,14 @@ void rte_timer_manage(void)
>  		priv_timer[lcore_id].running_tim = tim;
>  
>  		/* execute callback function with list unlocked */
> -		tim->f(tim, tim->arg);
> +		if (tim->period == 0)
> +			tim->f(tim, 1, tim->arg);
> +		else {
> +			/* for periodic check how many expiries we have */
> +			uint64_t over_time = cur_time - tim->expire;
> +			unsigned int extra_expiries = over_time / tim->period;
> +			tim->f(tim, 1 + extra_expiries, tim->arg);
> +		}
>  
>  		__TIMER_STAT_ADD(pending, -1);
>  		/* the timer was stopped or reloaded by the callback
> diff --git a/lib/librte_timer/rte_timer.h b/lib/librte_timer/rte_timer.h
> index a276a73..bc434ec 100644
> --- a/lib/librte_timer/rte_timer.h
> +++ b/lib/librte_timer/rte_timer.h
> @@ -117,7 +117,7 @@ struct rte_timer;
>  /**
>   * Callback function type for timer expiry.
>   */
> -typedef void (*rte_timer_cb_t)(struct rte_timer *, void *);
> +typedef void (*rte_timer_cb_t)(struct rte_timer *, unsigned int count, void *);
>  
>  #define MAX_SKIPLIST_DEPTH 10
>  
> diff --git a/test/test/test_timer.c b/test/test/test_timer.c
> index 2f6525a..0b86d3c 100644
> --- a/test/test/test_timer.c
> +++ b/test/test/test_timer.c
> @@ -153,7 +153,8 @@ struct mytimerinfo {
>  
>  static struct mytimerinfo mytiminfo[NB_TIMER];
>  
> -static void timer_basic_cb(struct rte_timer *tim, void *arg);
> +static void
> +timer_basic_cb(struct rte_timer *tim, unsigned int count, void *arg);
>  
>  static void
>  mytimer_reset(struct mytimerinfo *timinfo, uint64_t ticks,
> @@ -167,6 +168,7 @@ mytimer_reset(struct mytimerinfo *timinfo, uint64_t ticks,
>  /* timer callback for stress tests */
>  static void
>  timer_stress_cb(__attribute__((unused)) struct rte_timer *tim,
> +		__attribute__((unused)) unsigned int count,
>  		__attribute__((unused)) void *arg)
>  {
>  	long r;
> @@ -293,9 +295,11 @@ static volatile int cb_count = 0;
>  /* callback for second stress test. will only be called
>   * on master lcore */
>  static void
> -timer_stress2_cb(struct rte_timer *tim __rte_unused, void *arg __rte_unused)
> +timer_stress2_cb(struct rte_timer *tim __rte_unused,
> +		unsigned int count,
> +		void *arg __rte_unused)
>  {
> -	cb_count++;
> +	cb_count += count;
>  }
>  
>  #define NB_STRESS2_TIMERS 8192
> @@ -430,7 +434,7 @@ timer_stress2_main_loop(__attribute__((unused)) void *arg)
>  
>  /* timer callback for basic tests */
>  static void
> -timer_basic_cb(struct rte_timer *tim, void *arg)
> +timer_basic_cb(struct rte_timer *tim, unsigned int count, void *arg)
>  {
>  	struct mytimerinfo *timinfo = arg;
>  	uint64_t hz = rte_get_timer_hz();
> @@ -440,7 +444,7 @@ timer_basic_cb(struct rte_timer *tim, void *arg)
>  	if (rte_timer_pending(tim))
>  		return;
>  
> -	timinfo->count ++;
> +	timinfo->count += count;
>  
>  	RTE_LOG(INFO, TESTTIMER,
>  		"%"PRIu64": callback id=%u count=%u on core %u\n",
> diff --git a/test/test/test_timer_perf.c b/test/test/test_timer_perf.c
> index fa77efb..5b3867d 100644
> --- a/test/test/test_timer_perf.c
> +++ b/test/test/test_timer_perf.c
> @@ -48,7 +48,9 @@
>  int outstanding_count = 0;
>  
>  static void
> -timer_cb(struct rte_timer *t __rte_unused, void *param __rte_unused)
> +timer_cb(struct rte_timer *t __rte_unused,
> +		unsigned int count __rte_unused,
> +		void *param __rte_unused)
>  {
>  	outstanding_count--;
>  }
> diff --git a/test/test/test_timer_racecond.c b/test/test/test_timer_racecond.c
> index 7824ec4..b1c5c86 100644
> --- a/test/test/test_timer_racecond.c
> +++ b/test/test/test_timer_racecond.c
> @@ -65,7 +65,8 @@ static volatile unsigned stop_slaves;
>  static int reload_timer(struct rte_timer *tim);
>  
>  static void
> -timer_cb(struct rte_timer *tim, void *arg __rte_unused)
> +timer_cb(struct rte_timer *tim, unsigned int count __rte_unused,
> +		void *arg __rte_unused)
>  {
>  	/* Simulate slow callback function, 100 us. */
>  	rte_delay_us(100);
> -- 
> 2.9.3
>
  
Bruce Richardson May 31, 2017, 9:16 a.m. UTC | #3
For periodic timers, with the current there is no way to know if timer
expiries have been missed between calls to rte_timer_manage(). This
patchset adds in a new parameter to timer callbacks, to give the number of
expiries since the last one. ABI compatibility with previous releases is
kept, and a new unit test for that functionality is added

Bruce Richardson (3):
  timer: inform periodic timers of multiple expiries
  timer: add symbol versions for ABI compatibility
  test/test: add test for multiple timer expiries

 examples/l2fwd-jobstats/main.c                     |  7 +-
 examples/l2fwd-keepalive/main.c                    |  8 +-
 examples/l3fwd-power/main.c                        |  5 +-
 examples/performance-thread/common/lthread_sched.c |  4 +-
 examples/performance-thread/common/lthread_sched.h |  2 +-
 examples/timer/main.c                              | 10 ++-
 lib/librte_timer/rte_timer.c                       | 88 ++++++++++++++++++++--
 lib/librte_timer/rte_timer.h                       | 29 ++++++-
 lib/librte_timer/rte_timer_version.map             |  9 +++
 test/test/test_timer.c                             | 68 +++++++++++++++--
 test/test/test_timer_perf.c                        |  4 +-
 test/test/test_timer_racecond.c                    |  3 +-
 12 files changed, 203 insertions(+), 34 deletions(-)
  

Patch

diff --git a/examples/l2fwd-jobstats/main.c b/examples/l2fwd-jobstats/main.c
index e6e6c22..b264344 100644
--- a/examples/l2fwd-jobstats/main.c
+++ b/examples/l2fwd-jobstats/main.c
@@ -410,7 +410,8 @@  l2fwd_job_update_cb(struct rte_jobstats *job, int64_t result)
 }
 
 static void
-l2fwd_fwd_job(__rte_unused struct rte_timer *timer, void *arg)
+l2fwd_fwd_job(__rte_unused struct rte_timer *timer,
+		__rte_unused unsigned int count, void *arg)
 {
 	struct rte_mbuf *pkts_burst[MAX_PKT_BURST];
 	struct rte_mbuf *m;
@@ -460,7 +461,9 @@  l2fwd_fwd_job(__rte_unused struct rte_timer *timer, void *arg)
 }
 
 static void
-l2fwd_flush_job(__rte_unused struct rte_timer *timer, __rte_unused void *arg)
+l2fwd_flush_job(__rte_unused struct rte_timer *timer,
+		__rte_unused unsigned int count,
+		__rte_unused void *arg)
 {
 	uint64_t now;
 	unsigned lcore_id;
diff --git a/examples/l2fwd-keepalive/main.c b/examples/l2fwd-keepalive/main.c
index 4623d2a..26eba12 100644
--- a/examples/l2fwd-keepalive/main.c
+++ b/examples/l2fwd-keepalive/main.c
@@ -144,8 +144,9 @@  struct rte_keepalive *rte_global_keepalive_info;
 
 /* Print out statistics on packets dropped */
 static void
-print_stats(__attribute__((unused)) struct rte_timer *ptr_timer,
-	__attribute__((unused)) void *ptr_data)
+print_stats(__rte_unused struct rte_timer *ptr_timer,
+		__rte_unused unsigned int count,
+		__rte_unused void *ptr_data)
 {
 	uint64_t total_packets_dropped, total_packets_tx, total_packets_rx;
 	unsigned portid;
@@ -748,8 +749,7 @@  main(int argc, char **argv)
 				(check_period * rte_get_timer_hz()) / 1000,
 				PERIODICAL,
 				rte_lcore_id(),
-				(void(*)(struct rte_timer*, void*))
-				&rte_keepalive_dispatch_pings,
+				(void *)&rte_keepalive_dispatch_pings,
 				rte_global_keepalive_info
 				) != 0 )
 			rte_exit(EXIT_FAILURE, "Keepalive setup failure.\n");
diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index ec40a17..318aefd 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -410,8 +410,9 @@  signal_exit_now(int sigtype)
 
 /*  Freqency scale down timer callback */
 static void
-power_timer_cb(__attribute__((unused)) struct rte_timer *tim,
-			  __attribute__((unused)) void *arg)
+power_timer_cb(__rte_unused struct rte_timer *tim,
+		__rte_unused unsigned int count,
+		__rte_unused void *arg)
 {
 	uint64_t hz;
 	float sleep_time_ratio;
diff --git a/examples/performance-thread/common/lthread_sched.c b/examples/performance-thread/common/lthread_sched.c
index c64c21f..c4c7d3b 100644
--- a/examples/performance-thread/common/lthread_sched.c
+++ b/examples/performance-thread/common/lthread_sched.c
@@ -437,7 +437,9 @@  static inline void _lthread_resume(struct lthread *lt)
  * Handle sleep timer expiry
 */
 void
-_sched_timer_cb(struct rte_timer *tim, void *arg)
+_sched_timer_cb(struct rte_timer *tim,
+		unsigned int count __rte_unused,
+		void *arg)
 {
 	struct lthread *lt = (struct lthread *) arg;
 	uint64_t state = lt->state;
diff --git a/examples/performance-thread/common/lthread_sched.h b/examples/performance-thread/common/lthread_sched.h
index 7cddda9..f2af8b3 100644
--- a/examples/performance-thread/common/lthread_sched.h
+++ b/examples/performance-thread/common/lthread_sched.h
@@ -149,7 +149,7 @@  _reschedule(void)
 }
 
 extern struct lthread_sched *schedcore[];
-void _sched_timer_cb(struct rte_timer *tim, void *arg);
+void _sched_timer_cb(struct rte_timer *tim, unsigned int count, void *arg);
 void _sched_shutdown(__rte_unused void *arg);
 
 #ifdef __cplusplus
diff --git a/examples/timer/main.c b/examples/timer/main.c
index 37ad559..92a6a1f 100644
--- a/examples/timer/main.c
+++ b/examples/timer/main.c
@@ -55,8 +55,9 @@  static struct rte_timer timer1;
 
 /* timer0 callback */
 static void
-timer0_cb(__attribute__((unused)) struct rte_timer *tim,
-	  __attribute__((unused)) void *arg)
+timer0_cb(__rte_unused struct rte_timer *tim,
+	  __rte_unused unsigned int count,
+	  __rte_unused void *arg)
 {
 	static unsigned counter = 0;
 	unsigned lcore_id = rte_lcore_id();
@@ -71,8 +72,9 @@  timer0_cb(__attribute__((unused)) struct rte_timer *tim,
 
 /* timer1 callback */
 static void
-timer1_cb(__attribute__((unused)) struct rte_timer *tim,
-	  __attribute__((unused)) void *arg)
+timer1_cb(__rte_unused struct rte_timer *tim,
+	  __rte_unused unsigned int count,
+	  __rte_unused void *arg)
 {
 	unsigned lcore_id = rte_lcore_id();
 	uint64_t hz;
diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c
index 18782fa..d1e2c12 100644
--- a/lib/librte_timer/rte_timer.c
+++ b/lib/librte_timer/rte_timer.c
@@ -590,7 +590,14 @@  void rte_timer_manage(void)
 		priv_timer[lcore_id].running_tim = tim;
 
 		/* execute callback function with list unlocked */
-		tim->f(tim, tim->arg);
+		if (tim->period == 0)
+			tim->f(tim, 1, tim->arg);
+		else {
+			/* for periodic check how many expiries we have */
+			uint64_t over_time = cur_time - tim->expire;
+			unsigned int extra_expiries = over_time / tim->period;
+			tim->f(tim, 1 + extra_expiries, tim->arg);
+		}
 
 		__TIMER_STAT_ADD(pending, -1);
 		/* the timer was stopped or reloaded by the callback
diff --git a/lib/librte_timer/rte_timer.h b/lib/librte_timer/rte_timer.h
index a276a73..bc434ec 100644
--- a/lib/librte_timer/rte_timer.h
+++ b/lib/librte_timer/rte_timer.h
@@ -117,7 +117,7 @@  struct rte_timer;
 /**
  * Callback function type for timer expiry.
  */
-typedef void (*rte_timer_cb_t)(struct rte_timer *, void *);
+typedef void (*rte_timer_cb_t)(struct rte_timer *, unsigned int count, void *);
 
 #define MAX_SKIPLIST_DEPTH 10
 
diff --git a/test/test/test_timer.c b/test/test/test_timer.c
index 2f6525a..0b86d3c 100644
--- a/test/test/test_timer.c
+++ b/test/test/test_timer.c
@@ -153,7 +153,8 @@  struct mytimerinfo {
 
 static struct mytimerinfo mytiminfo[NB_TIMER];
 
-static void timer_basic_cb(struct rte_timer *tim, void *arg);
+static void
+timer_basic_cb(struct rte_timer *tim, unsigned int count, void *arg);
 
 static void
 mytimer_reset(struct mytimerinfo *timinfo, uint64_t ticks,
@@ -167,6 +168,7 @@  mytimer_reset(struct mytimerinfo *timinfo, uint64_t ticks,
 /* timer callback for stress tests */
 static void
 timer_stress_cb(__attribute__((unused)) struct rte_timer *tim,
+		__attribute__((unused)) unsigned int count,
 		__attribute__((unused)) void *arg)
 {
 	long r;
@@ -293,9 +295,11 @@  static volatile int cb_count = 0;
 /* callback for second stress test. will only be called
  * on master lcore */
 static void
-timer_stress2_cb(struct rte_timer *tim __rte_unused, void *arg __rte_unused)
+timer_stress2_cb(struct rte_timer *tim __rte_unused,
+		unsigned int count,
+		void *arg __rte_unused)
 {
-	cb_count++;
+	cb_count += count;
 }
 
 #define NB_STRESS2_TIMERS 8192
@@ -430,7 +434,7 @@  timer_stress2_main_loop(__attribute__((unused)) void *arg)
 
 /* timer callback for basic tests */
 static void
-timer_basic_cb(struct rte_timer *tim, void *arg)
+timer_basic_cb(struct rte_timer *tim, unsigned int count, void *arg)
 {
 	struct mytimerinfo *timinfo = arg;
 	uint64_t hz = rte_get_timer_hz();
@@ -440,7 +444,7 @@  timer_basic_cb(struct rte_timer *tim, void *arg)
 	if (rte_timer_pending(tim))
 		return;
 
-	timinfo->count ++;
+	timinfo->count += count;
 
 	RTE_LOG(INFO, TESTTIMER,
 		"%"PRIu64": callback id=%u count=%u on core %u\n",
diff --git a/test/test/test_timer_perf.c b/test/test/test_timer_perf.c
index fa77efb..5b3867d 100644
--- a/test/test/test_timer_perf.c
+++ b/test/test/test_timer_perf.c
@@ -48,7 +48,9 @@ 
 int outstanding_count = 0;
 
 static void
-timer_cb(struct rte_timer *t __rte_unused, void *param __rte_unused)
+timer_cb(struct rte_timer *t __rte_unused,
+		unsigned int count __rte_unused,
+		void *param __rte_unused)
 {
 	outstanding_count--;
 }
diff --git a/test/test/test_timer_racecond.c b/test/test/test_timer_racecond.c
index 7824ec4..b1c5c86 100644
--- a/test/test/test_timer_racecond.c
+++ b/test/test/test_timer_racecond.c
@@ -65,7 +65,8 @@  static volatile unsigned stop_slaves;
 static int reload_timer(struct rte_timer *tim);
 
 static void
-timer_cb(struct rte_timer *tim, void *arg __rte_unused)
+timer_cb(struct rte_timer *tim, unsigned int count __rte_unused,
+		void *arg __rte_unused)
 {
 	/* Simulate slow callback function, 100 us. */
 	rte_delay_us(100);