From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id DE65C3B5 for ; Fri, 28 Apr 2017 15:30:56 +0200 (CEST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 28 Apr 2017 06:30:55 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.37,388,1488873600"; d="scan'208";a="80101022" Received: from bricha3-mobl3.ger.corp.intel.com ([10.237.221.133]) by orsmga002.jf.intel.com with SMTP; 28 Apr 2017 06:30:53 -0700 Received: by (sSMTP sendmail emulation); Fri, 28 Apr 2017 14:30:53 +0100 Date: Fri, 28 Apr 2017 14:30:52 +0100 From: Bruce Richardson To: rsanford@akamai.com Cc: dev@dpdk.org Message-ID: <20170428133052.GB28692@bricha3-MOBL3.ger.corp.intel.com> References: <20170428132538.15995-1-bruce.richardson@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170428132538.15995-1-bruce.richardson@intel.com> Organization: Intel Research and =?iso-8859-1?Q?De=ACvel?= =?iso-8859-1?Q?opment?= Ireland Ltd. User-Agent: Mutt/1.8.0 (2017-02-23) Subject: Re: [dpdk-dev] [RFC PATCH] timer: inform periodic timers of multiple expiries X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 28 Apr 2017 13:30:57 -0000 +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 > --- > 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 >