[dpdk-dev] timer: remove unnecessary timer add call

Message ID 1468776950-32693-1-git-send-email-h.mikita89@gmail.com (mailing list archive)
State Accepted, archived
Headers

Commit Message

Hiroyuki Mikita July 17, 2016, 5:35 p.m. UTC
  When timer_set_running_state() fails in rte_timer_manage(),
the failed timer is put back on pending-list.
In this case, another core tries to reset or stop the timer.
It does not need to be on pending-list

Signed-off-by: Hiroyuki Mikita <h.mikita89@gmail.com>
---
 lib/librte_timer/rte_timer.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
  

Comments

Sanford, Robert July 22, 2016, 10:06 p.m. UTC | #1
On 7/17/16 1:35 PM, "Hiroyuki Mikita" <h.mikita89@gmail.com> wrote:

>When timer_set_running_state() fails in rte_timer_manage(),
>the failed timer is put back on pending-list.
>In this case, another core tries to reset or stop the timer.
>It does not need to be on pending-list
>
>Signed-off-by: Hiroyuki Mikita <h.mikita89@gmail.com>
>---
> lib/librte_timer/rte_timer.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
>diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c
>index 3dcdab5..94878d3 100644
>--- a/lib/librte_timer/rte_timer.c
>+++ b/lib/librte_timer/rte_timer.c
>@@ -562,10 +562,9 @@ void rte_timer_manage(void)
> 			pprev = &tim->sl_next[0];
> 		} else {
> 			/* another core is trying to re-config this one,
>-			 * remove it from local expired list and put it
>-			 * back on the priv_timer[] skip list */
>+			 * remove it from local expired list
>+			 */
> 			*pprev = next_tim;
>-			timer_add(tim, lcore_id, 1);
> 		}
> 	}
> 
>-- 
>2.7.4
>

Acked-by: Robert Sanford <rsanford@akamai.com>
  
Hiroyuki Mikita July 25, 2016, 3:15 p.m. UTC | #2
Fixes: a4b7a5a45cf5 ("timer: fix race condition")

2016-07-23 7:06 GMT+09:00 Sanford, Robert <rsanford@akamai.com>:
>
>
> On 7/17/16 1:35 PM, "Hiroyuki Mikita" <h.mikita89@gmail.com> wrote:
>
>>When timer_set_running_state() fails in rte_timer_manage(),
>>the failed timer is put back on pending-list.
>>In this case, another core tries to reset or stop the timer.
>>It does not need to be on pending-list
>>
>>Signed-off-by: Hiroyuki Mikita <h.mikita89@gmail.com>
>>---
>> lib/librte_timer/rte_timer.c | 5 ++---
>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>
>>diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c
>>index 3dcdab5..94878d3 100644
>>--- a/lib/librte_timer/rte_timer.c
>>+++ b/lib/librte_timer/rte_timer.c
>>@@ -562,10 +562,9 @@ void rte_timer_manage(void)
>>                       pprev = &tim->sl_next[0];
>>               } else {
>>                       /* another core is trying to re-config this one,
>>-                       * remove it from local expired list and put it
>>-                       * back on the priv_timer[] skip list */
>>+                       * remove it from local expired list
>>+                       */
>>                       *pprev = next_tim;
>>-                      timer_add(tim, lcore_id, 1);
>>               }
>>       }
>>
>>--
>>2.7.4
>>
>
> Acked-by: Robert Sanford <rsanford@akamai.com>
>
  
Thomas Monjalon July 25, 2016, 3:50 p.m. UTC | #3
2016-07-26 00:15, Hiroyuki Mikita:
> Fixes: a4b7a5a45cf5 ("timer: fix race condition")
> >
> >>When timer_set_running_state() fails in rte_timer_manage(),
> >>the failed timer is put back on pending-list.
> >>In this case, another core tries to reset or stop the timer.
> >>It does not need to be on pending-list
> >>
> >>Signed-off-by: Hiroyuki Mikita <h.mikita89@gmail.com>
> >
> > Acked-by: Robert Sanford <rsanford@akamai.com>

Applied, thanks
  

Patch

diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c
index 3dcdab5..94878d3 100644
--- a/lib/librte_timer/rte_timer.c
+++ b/lib/librte_timer/rte_timer.c
@@ -562,10 +562,9 @@  void rte_timer_manage(void)
 			pprev = &tim->sl_next[0];
 		} else {
 			/* another core is trying to re-config this one,
-			 * remove it from local expired list and put it
-			 * back on the priv_timer[] skip list */
+			 * remove it from local expired list
+			 */
 			*pprev = next_tim;
-			timer_add(tim, lcore_id, 1);
 		}
 	}