[3/4] timer: fix rte_timer_stop_all
Checks
Commit Message
there is a possibility of deadlock in this api,
as same spinlock is tried to be acquired in nested manner.
This patch removes the acquisition of nested locking.
Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
---
lib/timer/rte_timer.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
Comments
On Wed, 3 Aug 2022 11:26:51 -0500
Naga Harish K S V <s.v.naga.harish.k@intel.com> wrote:
> there is a possibility of deadlock in this api,
> as same spinlock is tried to be acquired in nested manner.
>
> This patch removes the acquisition of nested locking.
>
> Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
The wording in this commit message is a little confusing, what is
the exact lock conflict?
After your patch, there are no longer any callers for __rte_timer_stop()
with the local_is_locked flag. Please resubmit and remove all the
the local_is_locked from __rte_timer_stop().
It looks like the test suite for rte_timer is not exercising all
the exposed API's. That's a problem
Hi,
V2 of the patch is submitted with suggested changes in __rte_timer_stop() function.
-Harish
> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Wednesday, August 3, 2022 11:19 PM
> To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> Cc: Carrillo, Erik G <erik.g.carrillo@intel.com>; dev@dpdk.org
> Subject: Re: [PATCH 3/4] timer: fix rte_timer_stop_all
>
> On Wed, 3 Aug 2022 11:26:51 -0500
> Naga Harish K S V <s.v.naga.harish.k@intel.com> wrote:
>
> > there is a possibility of deadlock in this api, as same spinlock is
> > tried to be acquired in nested manner.
> >
> > This patch removes the acquisition of nested locking.
> >
> > Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
>
> The wording in this commit message is a little confusing, what is the exact
> lock conflict?
>
> After your patch, there are no longer any callers for c with
> the local_is_locked flag. Please resubmit and remove all the the
> local_is_locked from __rte_timer_stop().
>
> It looks like the test suite for rte_timer is not exercising all the exposed API's.
> That's a problem
@@ -987,21 +987,16 @@ rte_timer_stop_all(uint32_t timer_data_id, unsigned int *walk_lcores,
walk_lcore = walk_lcores[i];
priv_timer = &timer_data->priv_timer[walk_lcore];
- rte_spinlock_lock(&priv_timer->list_lock);
-
for (tim = priv_timer->pending_head.sl_next[0];
tim != NULL;
tim = next_tim) {
next_tim = tim->sl_next[0];
- /* Call timer_stop with lock held */
- __rte_timer_stop(tim, 1, timer_data);
+ __rte_timer_stop(tim, 0, timer_data);
if (f)
f(tim, f_arg);
}
-
- rte_spinlock_unlock(&priv_timer->list_lock);
}
return 0;