diff mbox

[ODP/PATCH,1/2] timer : add cancel_tmo function.

Message ID 1394889977-31193-1-git-send-email-santosh.shukla@linaro.org
State Superseded
Headers show

Commit Message

Santosh Shukla March 15, 2014, 1:26 p.m. UTC
From: santosh shukla <santosh.shukla@linaro.org>

Signed-off-by: santosh shukla <santosh.shukla@linaro.org>
---
 platform/linux-generic/source/odp_timer.c |   35 +++++++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

Comments

Petri Savolainen March 17, 2014, 8:28 a.m. UTC | #1
On Saturday, 15 March 2014 15:26:16 UTC+2, Santosh Shukla wrote:

> From: santosh shukla <santosh...@linaro.org <javascript:>> 
>
> Signed-off-by: santosh shukla <santosh...@linaro.org <javascript:>> 
> --- 
>  platform/linux-generic/source/odp_timer.c |   35 
> +++++++++++++++++++++++++++-- 
>  1 file changed, 33 insertions(+), 2 deletions(-) 
>
> diff --git a/platform/linux-generic/source/odp_timer.c 
> b/platform/linux-generic/source/odp_timer.c 
> index 4bcc479..b35d5e4 100644 
> --- a/platform/linux-generic/source/odp_timer.c 
> +++ b/platform/linux-generic/source/odp_timer.c 
> @@ -92,6 +92,39 @@ static timeout_t *rem_tmo(tick_t *tick) 
>  } 
>   
>   
> +/** 
> + * Cancel a timeout 
> + * 
> + * @param timer Timer 
> + * @param tmo   Timeout to cancel 
> + * 
> + * @return 0 if successful 
> + */ 
> +int odp_timer_cancel_tmo(odp_timer_t timer, odp_timer_tmo_t tmo) 
> +{ 
> +        int id; 
> +        uint64_t abs_tick; 
> +        timeout_t *new_tmo; 
> +        tick_t *tick; 
> + 
> +        /* get id */ 
> +        id = timer - 1; 
> + 
> +        /* get tmo_buf to cancel */ 
> +        new_tmo = (timeout_t *)odp_buffer_addr(tmo); 
> +        new_tmo->tmo_tick = 0; /* reset tmo */ 
> +        abs_tick = new_tmo->tick;  /* get the absolute 
> +                                        tick setted by prev add_tmo call 
> */ 
> + 
> +        tick = &odp_timer.timer[id].tick[abs_tick]; 
> +        if (rem_tmo(tick) == NULL) { 
> +                ODP_ERR("rem_tmo failed for timer-id %d        tmo-id :%d 
> tick-idx %lu\n", id, tmo, abs_tick); 
> +                return -1; 
> +        } 
>

Rem_tmo() removes the first tmo from the list. Instead, you have to first 
_find_ the tmo to be cancelled and then remove it (a tick may include a lot 
of other timeouts !). Second, you have to free the buffer.

-Petri
Santosh Shukla March 17, 2014, 8:50 a.m. UTC | #2
On 17 March 2014 13:58, Petri Savolainen <petri.savolainen@linaro.org> wrote:
>
>
> On Saturday, 15 March 2014 15:26:16 UTC+2, Santosh Shukla wrote:
>>
>> From: santosh shukla <santosh...@linaro.org>
>>
>> Signed-off-by: santosh shukla <santosh...@linaro.org>
>> ---
>>  platform/linux-generic/source/odp_timer.c |   35
>> +++++++++++++++++++++++++++--
>>  1 file changed, 33 insertions(+), 2 deletions(-)
>>
>> diff --git a/platform/linux-generic/source/odp_timer.c
>> b/platform/linux-generic/source/odp_timer.c
>> index 4bcc479..b35d5e4 100644
>> --- a/platform/linux-generic/source/odp_timer.c
>> +++ b/platform/linux-generic/source/odp_timer.c
>> @@ -92,6 +92,39 @@ static timeout_t *rem_tmo(tick_t *tick)
>>  }
>>
>>
>> +/**
>> + * Cancel a timeout
>> + *
>> + * @param timer Timer
>> + * @param tmo   Timeout to cancel
>> + *
>> + * @return 0 if successful
>> + */
>> +int odp_timer_cancel_tmo(odp_timer_t timer, odp_timer_tmo_t tmo)
>> +{
>> +        int id;
>> +        uint64_t abs_tick;
>> +        timeout_t *new_tmo;
>> +        tick_t *tick;
>> +
>> +        /* get id */
>> +        id = timer - 1;
>> +
>> +        /* get tmo_buf to cancel */
>> +        new_tmo = (timeout_t *)odp_buffer_addr(tmo);
>> +        new_tmo->tmo_tick = 0; /* reset tmo */
>> +        abs_tick = new_tmo->tick;  /* get the absolute
>> +                                        tick setted by prev add_tmo call
>> */
>> +
>> +        tick = &odp_timer.timer[id].tick[abs_tick];
>> +        if (rem_tmo(tick) == NULL) {
>> +                ODP_ERR("rem_tmo failed for timer-id %d        tmo-id :%d
>> tick-idx %lu\n", id, tmo, abs_tick);
>> +                return -1;
>> +        }
>
>
> Rem_tmo() removes the first tmo from the list. Instead, you have to first
> _find_ the tmo to be cancelled and then remove it (a tick may include a lot
> of other timeouts !). Second, you have to free the buffer.

confused ? finding tmo is nothing but code does..right. What I missed
is - deleting specific timeout from tmo list but then I dont have any
user input stating specific timeout.. any idea?

>
> -Petri
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "LNG ODP Sub-team - lng-odp@linaro.org" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to lng-odp+unsubscribe@linaro.org.
> To post to this group, send email to lng-odp@linaro.org.
> Visit this group at http://groups.google.com/a/linaro.org/group/lng-odp/.
> To view this discussion on the web visit
> https://groups.google.com/a/linaro.org/d/msgid/lng-odp/21bbe77f-6f51-4010-80b6-a61d6e579178%40linaro.org.
> For more options, visit https://groups.google.com/a/linaro.org/d/optout.
Petri Savolainen March 17, 2014, 9:01 a.m. UTC | #3
> > Rem_tmo() removes the first tmo from the list. Instead, you have to 
> first 
> > _find_ the tmo to be cancelled and then remove it (a tick may include a 
> lot 
> > of other timeouts !). Second, you have to free the buffer. 
>
> confused ? finding tmo is nothing but code does..right. What I missed 
> is - deleting specific timeout from tmo list but then I dont have any 
> user input stating specific timeout.. any idea? 
>
>  
Each tick has a linked list of timeouts. You have to search through that 
list for a tmo that matches the "tmo" handle user gives in the cancel call. 

-Petri
Santosh Shukla March 17, 2014, 9:27 a.m. UTC | #4
On 17 March 2014 14:31, Petri Savolainen <petri.savolainen@linaro.org> wrote:
>
>>
>> > Rem_tmo() removes the first tmo from the list. Instead, you have to
>> > first
>> > _find_ the tmo to be cancelled and then remove it (a tick may include a
>> > lot
>> > of other timeouts !). Second, you have to free the buffer.
>>
>> confused ? finding tmo is nothing but code does..right. What I missed
>> is - deleting specific timeout from tmo list but then I dont have any
>> user input stating specific timeout.. any idea?
>>
>
> Each tick has a linked list of timeouts. You have to search through that
> list for a tmo that matches the "tmo" handle user gives in the cancel call.
>

Understood and thanks for confirming.. I have one another question.

why we need odp_buffer_t buf in timeout, we already have tmo_buf for
timeout case. Also do cancel method should consider them for
cancelling timeout?

If no need to cancel odp_buffer_t buf in cancel method then my
modified code will look this.. is it Ok?
tick = &odp_timer.timer[id].tick[abs_tick];

        while ((new_tmo=rem_tmo(tick)) != NULL) {
                if (new_tmo->tmo_buf == tmo) {
                        odp_buffer_free(new_tmo->tmo_buf);
                        break;
                }
        }

        if (new_buf == NULL) {
                ODP_ERR("rem_tmo failed for timer-id %d tmo-id :%d
tick-idx %lu\n", id, tmo, abs_tick);
                return -1;
        }

Thanks.

> -Petri
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "LNG ODP Sub-team - lng-odp@linaro.org" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to lng-odp+unsubscribe@linaro.org.
> To post to this group, send email to lng-odp@linaro.org.
> Visit this group at http://groups.google.com/a/linaro.org/group/lng-odp/.
> To view this discussion on the web visit
> https://groups.google.com/a/linaro.org/d/msgid/lng-odp/f0f58892-1882-4339-807d-da8f30fa0077%40linaro.org.
>
> For more options, visit https://groups.google.com/a/linaro.org/d/optout.
Ola Liljedahl March 17, 2014, 12:09 p.m. UTC | #5
Guys!

Before you implement too much, I want to complete the discussion on the
timer API. I had some issues with Leo's proposal which he tried to answer.
This new API from Petri hasn't been discussed at all and I have *many*
issues with it. Why do you start to code before you know what to code?

-- Ola




On 17 March 2014 10:27, Santosh Shukla <santosh.shukla@linaro.org> wrote:

> On 17 March 2014 14:31, Petri Savolainen <petri.savolainen@linaro.org>
> wrote:
> >
> >>
> >> > Rem_tmo() removes the first tmo from the list. Instead, you have to
> >> > first
> >> > _find_ the tmo to be cancelled and then remove it (a tick may include
> a
> >> > lot
> >> > of other timeouts !). Second, you have to free the buffer.
> >>
> >> confused ? finding tmo is nothing but code does..right. What I missed
> >> is - deleting specific timeout from tmo list but then I dont have any
> >> user input stating specific timeout.. any idea?
> >>
> >
> > Each tick has a linked list of timeouts. You have to search through that
> > list for a tmo that matches the "tmo" handle user gives in the cancel
> call.
> >
>
> Understood and thanks for confirming.. I have one another question.
>
> why we need odp_buffer_t buf in timeout, we already have tmo_buf for
> timeout case. Also do cancel method should consider them for
> cancelling timeout?
>
> If no need to cancel odp_buffer_t buf in cancel method then my
> modified code will look this.. is it Ok?
> tick = &odp_timer.timer[id].tick[abs_tick];
>
>         while ((new_tmo=rem_tmo(tick)) != NULL) {
>                 if (new_tmo->tmo_buf == tmo) {
>                         odp_buffer_free(new_tmo->tmo_buf);
>                         break;
>                 }
>         }
>
>         if (new_buf == NULL) {
>                 ODP_ERR("rem_tmo failed for timer-id %d tmo-id :%d
> tick-idx %lu\n", id, tmo, abs_tick);
>                 return -1;
>         }
>
> Thanks.
>
> > -Petri
> >
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> > "LNG ODP Sub-team - lng-odp@linaro.org" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> > email to lng-odp+unsubscribe@linaro.org.
> > To post to this group, send email to lng-odp@linaro.org.
> > Visit this group at http://groups.google.com/a/linaro.org/group/lng-odp/
> .
> > To view this discussion on the web visit
> >
> https://groups.google.com/a/linaro.org/d/msgid/lng-odp/f0f58892-1882-4339-807d-da8f30fa0077%40linaro.org
> .
> >
> > For more options, visit https://groups.google.com/a/linaro.org/d/optout.
>
> --
> You received this message because you are subscribed to the Google Groups
> "LNG ODP Sub-team - lng-odp@linaro.org" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to lng-odp+unsubscribe@linaro.org.
> To post to this group, send email to lng-odp@linaro.org.
> Visit this group at http://groups.google.com/a/linaro.org/group/lng-odp/.
> To view this discussion on the web visit
> https://groups.google.com/a/linaro.org/d/msgid/lng-odp/CA%2BiXiiNCLOXeNf45c5kSDHEqTzRBmUgpf20fZjbGweRVKjcW9Q%40mail.gmail.com
> .
> For more options, visit https://groups.google.com/a/linaro.org/d/optout.
>
Santosh Shukla March 17, 2014, 12:11 p.m. UTC | #6
Ola,

We are in lng-odp call right now :) We can discuss.. Also post your
timer issues/comment on old thread.

Thanks.

On 17 March 2014 17:39, Ola Liljedahl <ola.liljedahl@linaro.org> wrote:
> Guys!
>
> Before you implement too much, I want to complete the discussion on the
> timer API. I had some issues with Leo's proposal which he tried to answer.
> This new API from Petri hasn't been discussed at all and I have *many*
> issues with it. Why do you start to code before you know what to code?
>
> -- Ola
>
>
>
>
> On 17 March 2014 10:27, Santosh Shukla <santosh.shukla@linaro.org> wrote:
>>
>> On 17 March 2014 14:31, Petri Savolainen <petri.savolainen@linaro.org>
>> wrote:
>> >
>> >>
>> >> > Rem_tmo() removes the first tmo from the list. Instead, you have to
>> >> > first
>> >> > _find_ the tmo to be cancelled and then remove it (a tick may include
>> >> > a
>> >> > lot
>> >> > of other timeouts !). Second, you have to free the buffer.
>> >>
>> >> confused ? finding tmo is nothing but code does..right. What I missed
>> >> is - deleting specific timeout from tmo list but then I dont have any
>> >> user input stating specific timeout.. any idea?
>> >>
>> >
>> > Each tick has a linked list of timeouts. You have to search through that
>> > list for a tmo that matches the "tmo" handle user gives in the cancel
>> > call.
>> >
>>
>> Understood and thanks for confirming.. I have one another question.
>>
>> why we need odp_buffer_t buf in timeout, we already have tmo_buf for
>> timeout case. Also do cancel method should consider them for
>> cancelling timeout?
>>
>> If no need to cancel odp_buffer_t buf in cancel method then my
>> modified code will look this.. is it Ok?
>> tick = &odp_timer.timer[id].tick[abs_tick];
>>
>>         while ((new_tmo=rem_tmo(tick)) != NULL) {
>>                 if (new_tmo->tmo_buf == tmo) {
>>                         odp_buffer_free(new_tmo->tmo_buf);
>>                         break;
>>                 }
>>         }
>>
>>         if (new_buf == NULL) {
>>                 ODP_ERR("rem_tmo failed for timer-id %d tmo-id :%d
>> tick-idx %lu\n", id, tmo, abs_tick);
>>                 return -1;
>>         }
>>
>> Thanks.
>>
>> > -Petri
>> >
>> >
>> > --
>> > You received this message because you are subscribed to the Google
>> > Groups
>> > "LNG ODP Sub-team - lng-odp@linaro.org" group.
>> > To unsubscribe from this group and stop receiving emails from it, send
>> > an
>> > email to lng-odp+unsubscribe@linaro.org.
>> > To post to this group, send email to lng-odp@linaro.org.
>> > Visit this group at
>> > http://groups.google.com/a/linaro.org/group/lng-odp/.
>> > To view this discussion on the web visit
>> >
>> > https://groups.google.com/a/linaro.org/d/msgid/lng-odp/f0f58892-1882-4339-807d-da8f30fa0077%40linaro.org.
>> >
>> > For more options, visit https://groups.google.com/a/linaro.org/d/optout.
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "LNG ODP Sub-team - lng-odp@linaro.org" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to lng-odp+unsubscribe@linaro.org.
>> To post to this group, send email to lng-odp@linaro.org.
>> Visit this group at http://groups.google.com/a/linaro.org/group/lng-odp/.
>> To view this discussion on the web visit
>> https://groups.google.com/a/linaro.org/d/msgid/lng-odp/CA%2BiXiiNCLOXeNf45c5kSDHEqTzRBmUgpf20fZjbGweRVKjcW9Q%40mail.gmail.com.
>>
>> For more options, visit https://groups.google.com/a/linaro.org/d/optout.
>
>
diff mbox

Patch

diff --git a/platform/linux-generic/source/odp_timer.c b/platform/linux-generic/source/odp_timer.c
index 4bcc479..b35d5e4 100644
--- a/platform/linux-generic/source/odp_timer.c
+++ b/platform/linux-generic/source/odp_timer.c
@@ -92,6 +92,39 @@  static timeout_t *rem_tmo(tick_t *tick)
 }
 
 
+/**
+ * Cancel a timeout
+ *
+ * @param timer Timer
+ * @param tmo   Timeout to cancel
+ *
+ * @return 0 if successful
+ */
+int odp_timer_cancel_tmo(odp_timer_t timer, odp_timer_tmo_t tmo)
+{
+	int id;
+	uint64_t abs_tick;
+	timeout_t *new_tmo;
+	tick_t *tick;
+
+	/* get id */
+	id = timer - 1;
+
+	/* get tmo_buf to cancel */
+	new_tmo = (timeout_t *)odp_buffer_addr(tmo);
+	new_tmo->tmo_tick = 0; /* reset tmo */
+	abs_tick = new_tmo->tick;  /* get the absolute
+					tick setted by prev add_tmo call */
+
+	tick = &odp_timer.timer[id].tick[abs_tick];
+	if (rem_tmo(tick) == NULL) {
+		ODP_ERR("rem_tmo failed for timer-id %d	tmo-id :%d tick-idx %lu\n", id, tmo, abs_tick);
+		return -1;
+	}
+
+	return 0;
+}
+
 
 static void notify_function(union sigval sigval)
 {
@@ -167,8 +200,6 @@  int odp_timer_init_global(void)
 		odp_spinlock_init(&odp_timer.timer[0].tick[i].lock);
 
 	timer_init();
-
-
 	return 0;
 }