On 11 March 2015 at 15:26, Ola Liljedahl <[email protected]> wrote:

> On 11 March 2015 at 14:49, Savolainen, Petri (Nokia - FI/Espoo) <
> [email protected]> wrote:
>
>>
>>
>> > -----Original Message-----
>> > From: [email protected] [mailto:lng-odp-
>> > [email protected]] On Behalf Of ext Ola Liljedahl
>> > Sent: Tuesday, March 10, 2015 4:44 PM
>> > To: [email protected]
>> > Subject: [lng-odp] [PATCH] linux-generic: odp_timer: set user_ptr for
>> > cancelled timeout
>> >
>> > Ensure that the timeout user_ptr and timer fields are set when the
>> > corresponding timer is immediately cancelled.
>> > https://bugs.linaro.org/show_bug.cgi?id=1313
>> >
>> > Signed-off-by: Ola Liljedahl <[email protected]>
>> > ---
>> > (This document/code contribution attached is provided under the terms of
>> > agreement LES-LTM-21309)
>> >
>> > Passes odp_timer validation with the new odp_timer_cancel() test from
>> > Petri.
>> >
>> >  platform/linux-generic/odp_timer.c | 18 ++++++++++++++----
>> >  1 file changed, 14 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/platform/linux-generic/odp_timer.c b/platform/linux-
>> > generic/odp_timer.c
>> > index 61a02b6..6b48d2e 100644
>> > --- a/platform/linux-generic/odp_timer.c
>> > +++ b/platform/linux-generic/odp_timer.c
>> > @@ -421,6 +421,16 @@ static bool timer_reset(uint32_t idx,
>> >  #endif
>> >       } else {
>> >               /* We have a new timeout buffer which replaces any old
>> one */
>> > +             /* Fill in header fields if timeout event */
>>
>> Typo: "... if timeout event" => "... in timeout event"
>>
> Will fix.
>
Now looking at what I wrote I actually wrote what I meant. But I should
probably have phrased it like this: "If timeout event then fill in headers"
or "When timeout event fill in headers". I hope this would be clearer.
The specified event does not have to be a timeout event.



>
> BTW should the timer implementation still be using buffers as the base
> class for timeouts? I think there is some more event-ification work to do
> here.
>
>
>> > +             if (_odp_buffer_type(*tmo_buf) == ODP_EVENT_TIMEOUT) {
>> > +                     /* Convert from buffer to timeout hdr */
>> > +                     odp_timeout_hdr_t *tmo_hdr =
>> > +                             timeout_hdr_from_buf(*tmo_buf);
>> > +                     tmo_hdr->timer = tp_idx_to_handle(tp, idx);
>> > +                     tmo_hdr->user_ptr = tp->timers[idx].user_ptr;
>> > +                     /* expiration field filled in when timer expires
>> */
>> > +             }
>> > +             /* Else ignore buffers of other types */
>> >               odp_buffer_t old_buf = ODP_BUFFER_INVALID;
>> >  #ifdef ODP_ATOMIC_U128
>> >               tick_buf_t new, old;
>> > @@ -556,16 +566,16 @@ static unsigned timer_expire(odp_timer_pool *tp,
>> > uint32_t idx, uint64_t tick)
>> >       _odp_atomic_flag_clear(IDX2LOCK(idx));
>> >  #endif
>> >       if (odp_likely(tmo_buf != ODP_BUFFER_INVALID)) {
>> > -             /* Fill in metadata fields in system timeout buffer */
>> > +             /* Fill in expiration tick if timeout event */
>>
>> Same typo.
>>
> Will fix.
>
>
>>
>> -Petri
>>
>> >               if (_odp_buffer_type(tmo_buf) == ODP_EVENT_TIMEOUT) {
>> >                       /* Convert from buffer to timeout hdr */
>> >                       odp_timeout_hdr_t *tmo_hdr =
>> >                               timeout_hdr_from_buf(tmo_buf);
>> > -                     tmo_hdr->timer = tp_idx_to_handle(tp, idx);
>> >                       tmo_hdr->expiration = exp_tck;
>> > -                     tmo_hdr->user_ptr = tim->user_ptr;
>> > +                     /* timer and user_ptr fields filled in when timer
>> > +                      * was set */
>> >               }
>> > -             /* Else ignore buffers of other types */
>> > +             /* Else ignore events of other types */
>> >               /* Post the timeout to the destination queue */
>> >               int rc = odp_queue_enq(tim->queue,
>> >                                      odp_buffer_to_event(tmo_buf));
>> > --
>> > 1.9.1
>> >
>> >
>> > _______________________________________________
>> > lng-odp mailing list
>> > [email protected]
>> > http://lists.linaro.org/mailman/listinfo/lng-odp
>>
>
>
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to