* [dpdk-dev] [PATCH] timer bug fix
@ 2014-05-21 19:53 Vadim Suraev
2014-05-23 14:52 ` Olivier MATZ
0 siblings, 1 reply; 6+ messages in thread
From: Vadim Suraev @ 2014-05-21 19:53 UTC (permalink / raw)
To: dev
Bug: when a periodic timer's callback is running, if another
timer is manipulated, the periodic timer is not reloaded.
Solution: set the update flag only is the modified timer is
in RUNNING state
Signed-off-by: Vadim Suraev <vadim.suraev@gmail.com>
---
lib/librte_timer/rte_timer.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c
index 884ee0e..1ebd223 100755
--- a/lib/librte_timer/rte_timer.c
+++ b/lib/librte_timer/rte_timer.c
@@ -378,7 +378,9 @@ __rte_timer_reset(struct rte_timer *tim, uint64_t expire,
return -1;
__TIMER_STAT_ADD(reset, 1);
- priv_timer[lcore_id].updated = 1;
+ if(prev_status.state == RTE_TIMER_RUNNING) {
+ priv_timer[lcore_id].updated = 1;
+ }
/* remove it from list */
if (prev_status.state == RTE_TIMER_PENDING) {
@@ -453,7 +455,9 @@ rte_timer_stop(struct rte_timer *tim)
return -1;
__TIMER_STAT_ADD(stop, 1);
- priv_timer[lcore_id].updated = 1;
+ if(prev_status.state == RTE_TIMER_RUNNING) {
+ priv_timer[lcore_id].updated = 1;
+ }
/* remove it from list */
if (prev_status.state == RTE_TIMER_PENDING) {
--
1.7.9.5
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] timer bug fix
2014-05-21 19:53 [dpdk-dev] [PATCH] timer bug fix Vadim Suraev
@ 2014-05-23 14:52 ` Olivier MATZ
2014-05-26 16:24 ` Thomas Monjalon
0 siblings, 1 reply; 6+ messages in thread
From: Olivier MATZ @ 2014-05-23 14:52 UTC (permalink / raw)
To: Vadim Suraev, dev
Hi Vadim,
It's even more simple that what I've suggested. It should
work since the only case where state is RTE_TIMER_RUNNING
is that we are modifying the timer currently running on the
same lcore. Indeed, timer_set_config_state() prevents us to
modify a running timer belonging to another lcore.
Just 3 small typos:
On 05/21/2014 09:53 PM, Vadim Suraev wrote:
> Bug: when a periodic timer's callback is running, if another
> timer is manipulated, the periodic timer is not reloaded.
> Solution: set the update flag only is the modified timer is
> in RUNNING state
s/is the modified/if the modified
> - priv_timer[lcore_id].updated = 1;
> + if(prev_status.state == RTE_TIMER_RUNNING) {
> + priv_timer[lcore_id].updated = 1;
> + }
missing a space after the if.
> - priv_timer[lcore_id].updated = 1;
> + if(prev_status.state == RTE_TIMER_RUNNING) {
> + priv_timer[lcore_id].updated = 1;
> + }
same here.
Acked-by: Olivier Matz <olivier.matz@6wind.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] timer bug fix
2014-05-23 14:52 ` Olivier MATZ
@ 2014-05-26 16:24 ` Thomas Monjalon
0 siblings, 0 replies; 6+ messages in thread
From: Thomas Monjalon @ 2014-05-26 16:24 UTC (permalink / raw)
To: Vadim Suraev; +Cc: dev
2014-05-23 16:52, Olivier MATZ:
> > Bug: when a periodic timer's callback is running, if another
> > timer is manipulated, the periodic timer is not reloaded.
> > Solution: set the update flag only is the modified timer is
> > in RUNNING state
>
> Acked-by: Olivier Matz <olivier.matz@6wind.com>
Applied in version 1.7.0 with title:
timer: fix reloading after changes
Thanks
--
Thomas
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] timer bug fix.
2014-05-23 14:44 ` Olivier MATZ
@ 2014-05-26 16:25 ` Thomas Monjalon
0 siblings, 0 replies; 6+ messages in thread
From: Thomas Monjalon @ 2014-05-26 16:25 UTC (permalink / raw)
To: Vadim Suraev; +Cc: dev
> > Bug: When a timer is running
> >
> > - if rte_timer_stop is called, the pending decrement is
> > skipped (decremented only if the timer is pending) and due
> > to the update flag the future processing is skipped so the
> > timer is counted as pending while it is stopped. - the same
> > applies when rte_timer_reset is called but then the pending
> > statistics is additionally incremented so the timer is
> > counted pending twice.
> >
> > Solution: decrement the pending
> >
> > statistics after returning from the callback. If
> > rte_timer_stop was called, it skipped decrementing the
> > pending statistics. If rte_time_reset was called, the
> > pending statistics was incremented. If neither was called
> > and the timer is periodic, the pending statistics is
> > incremented when it is reloaded
> >
> > Signed-off-by: Vadim Suraev <vadim.suraev@gmail.com>
>
> Acked-by: Olivier Matz <olivier.matz@6wind.com>
Applied for version 1.7.0 with title:
timer: fix pending counter
Thanks
--
Thomas
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] timer bug fix.
2014-05-21 20:35 Vadim Suraev
@ 2014-05-23 14:44 ` Olivier MATZ
2014-05-26 16:25 ` Thomas Monjalon
0 siblings, 1 reply; 6+ messages in thread
From: Olivier MATZ @ 2014-05-23 14:44 UTC (permalink / raw)
To: Vadim Suraev, dev
Acked-by: Olivier Matz <olivier.matz@6wind.com>
On 05/21/2014 10:35 PM, Vadim Suraev wrote:
> Bug: When a timer is running
> - if rte_timer_stop is called, the pending decrement is
> skipped (decremented only if the timer is pending) and due
> to the update flag the future processing is skipped so the
> timer is counted as pending while it is stopped. - the same
> applies when rte_timer_reset is called but then the pending
> statistics is additionally incremented so the timer is
> counted pending twice.
> Solution: decrement the pending
> statistics after returning from the callback. If
> rte_timer_stop was called, it skipped decrementing the
> pending statistics. If rte_time_reset was called, the
> pending statistics was incremented. If neither was called
> and the timer is periodic, the pending statistics is
> incremented when it is reloaded
>
>
> Signed-off-by: Vadim Suraev <vadim.suraev@gmail.com>
> ---
> lib/librte_timer/rte_timer.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c
> index 1ebd223..7035bed 100755
> --- a/lib/librte_timer/rte_timer.c
> +++ b/lib/librte_timer/rte_timer.c
> @@ -551,7 +551,7 @@ void rte_timer_manage(void)
> tim->f(tim, tim->arg);
>
> rte_spinlock_lock(&priv_timer[lcore_id].list_lock);
> -
> + __TIMER_STAT_ADD(pending, -1);
> /* the timer was stopped or reloaded by the callback
> * function, we have nothing to do here */
> if (priv_timer[lcore_id].updated == 1)
> @@ -559,7 +559,6 @@ void rte_timer_manage(void)
>
> if (tim->period == 0) {
> /* remove from done list and mark timer as stopped */
> - __TIMER_STAT_ADD(pending, -1);
> status.state = RTE_TIMER_STOP;
> status.owner = RTE_TIMER_NO_OWNER;
> rte_wmb();
> @@ -568,6 +567,7 @@ void rte_timer_manage(void)
> else {
> /* keep it in list and mark timer as pending */
> status.state = RTE_TIMER_PENDING;
> + __TIMER_STAT_ADD(pending, 1);
> status.owner = (int16_t)lcore_id;
> rte_wmb();
> tim->status.u32 = status.u32;
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [dpdk-dev] [PATCH] timer bug fix.
@ 2014-05-21 20:35 Vadim Suraev
2014-05-23 14:44 ` Olivier MATZ
0 siblings, 1 reply; 6+ messages in thread
From: Vadim Suraev @ 2014-05-21 20:35 UTC (permalink / raw)
To: dev
Bug: When a timer is running
- if rte_timer_stop is called, the pending decrement is
skipped (decremented only if the timer is pending) and due
to the update flag the future processing is skipped so the
timer is counted as pending while it is stopped. - the same
applies when rte_timer_reset is called but then the pending
statistics is additionally incremented so the timer is
counted pending twice.
Solution: decrement the pending
statistics after returning from the callback. If
rte_timer_stop was called, it skipped decrementing the
pending statistics. If rte_time_reset was called, the
pending statistics was incremented. If neither was called
and the timer is periodic, the pending statistics is
incremented when it is reloaded
Signed-off-by: Vadim Suraev <vadim.suraev@gmail.com>
---
lib/librte_timer/rte_timer.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c
index 1ebd223..7035bed 100755
--- a/lib/librte_timer/rte_timer.c
+++ b/lib/librte_timer/rte_timer.c
@@ -551,7 +551,7 @@ void rte_timer_manage(void)
tim->f(tim, tim->arg);
rte_spinlock_lock(&priv_timer[lcore_id].list_lock);
-
+ __TIMER_STAT_ADD(pending, -1);
/* the timer was stopped or reloaded by the callback
* function, we have nothing to do here */
if (priv_timer[lcore_id].updated == 1)
@@ -559,7 +559,6 @@ void rte_timer_manage(void)
if (tim->period == 0) {
/* remove from done list and mark timer as stopped */
- __TIMER_STAT_ADD(pending, -1);
status.state = RTE_TIMER_STOP;
status.owner = RTE_TIMER_NO_OWNER;
rte_wmb();
@@ -568,6 +567,7 @@ void rte_timer_manage(void)
else {
/* keep it in list and mark timer as pending */
status.state = RTE_TIMER_PENDING;
+ __TIMER_STAT_ADD(pending, 1);
status.owner = (int16_t)lcore_id;
rte_wmb();
tim->status.u32 = status.u32;
--
1.7.9.5
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-05-26 16:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-21 19:53 [dpdk-dev] [PATCH] timer bug fix Vadim Suraev
2014-05-23 14:52 ` Olivier MATZ
2014-05-26 16:24 ` Thomas Monjalon
2014-05-21 20:35 Vadim Suraev
2014-05-23 14:44 ` Olivier MATZ
2014-05-26 16:25 ` Thomas Monjalon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).