ping. Are we expecting a v2 of this patch?
On Sun, Apr 17, 2016 at 2:58 PM, Bill Fischofer <bill.fischo...@linaro.org>
wrote:
> General comment: This explanation could benefit considerably from the
> inclusion of a state diagram like that used for the pktio documentation.
> Here's the suggested timer_fsm.gv file:
>
> digraph timer_state_machine {
> rankdir=LR;
> size="10,12";
> node [fontsize=28];
> edge [fontsize=28];
> node [shape=doublecircle]; Timer_Unalloc
> Timeout_Unalloc
> Timeout_Delivered;
> node [shape=circle];
> Timer_Unalloc -> Timer_Alloc [label="odp_timer_alloc()"];
> Timer_Alloc -> Timer_Unalloc [label="odp_timer_free()"];
> Timer_Alloc -> Timer_Set [label="odp_timer_set_abs()"];
> Timer_Alloc -> Timer_Set [label="odp_timer_set_rel()"];
> Timer_Set -> Timer_Alloc [label="odp_timer_cancel()"];
> Timer_Set -> Timeout_Alloc
> [label="odp_timer_cancel()" constraint=false];
> Timer_Set -> Timer_Fired [label="(timer expires)"];
> Timer_Fired -> Timeout_Delivered [label="odp_schedule()"];
> Timeout_Unalloc -> Timeout_Alloc
> [label="odp_timeout_alloc()" constraint=false];
> Timeout_Alloc -> Timeout_Unalloc
> [label="odp_timeout_free()" constraint=false];
> Timeout_Alloc -> Timer_Set
> [label="odp_timer_set_abs()" constraint=false];
> Timeout_Alloc -> Timer_Set
> [label="odp_timer_set_rel()"];
> Timeout_Delivered -> Timer_Unalloc [label="odp_timer_free()"];
> Timeout_Delivered -> Timer_Set [label="odp_timer_set_abs()"];
> Timeout_Delivered -> Timer_Set [label="odp_timer_set_rel()"];
> Timeout_Delivered -> Timeout_Delivered
> [label="odp_timeout_from_event()"];
> Timeout_Delivered -> Timeout_Delivered
> [label="odp_timeout_timer()"];
> Timeout_Delivered -> Timeout_Unalloc
> [label="odp_event_free() / odp_timeout_freee()"
> constraint=false];
> }
>
> This should be added to doc/images and the .svg file referenced from there
> and should help clarify the text in explaining the relationship between
> timers and timeouts.
>
> Additional suggestions/corrections inline:
>
> On Thu, Apr 14, 2016 at 8:31 AM, Ivan Khoronzhuk <
> ivan.khoronz...@linaro.org> wrote:
>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronz...@linaro.org>
>> ---
>> v2..v1:
>> - just rebased with several not important corrections
>>
>> doc/users-guide/users-guide.adoc | 50
>> ++++++++++++++++++++++++++++++++++++----
>> 1 file changed, 45 insertions(+), 5 deletions(-)
>>
>> diff --git a/doc/users-guide/users-guide.adoc
>> b/doc/users-guide/users-guide.adoc
>> index a2e5058..69b1930 100644
>> --- a/doc/users-guide/users-guide.adoc
>> +++ b/doc/users-guide/users-guide.adoc
>> @@ -335,11 +335,51 @@ The +odp_time_t+ opaque type represents local or
>> global timestamps.
>>
>> === Timer
>> Timers are how ODP applications measure and respond to the passage of
>> time.
>> -Timers are drawn from specialized pools called timer pools that have
>> their
>> -own abstract type (+odp_timer_pool_t+). Applications may have many timers
>> -active at the same time and can set them to use either relative or
>> absolute
>> -time. When timers expire they create events of type +odp_timeout_t+,
>> which
>> -serve as notifications of timer expiration.
>> +The timer API is supposed to be used when time synchronization with
>> events is
>>
>
> delete 'supposed to be' (extraneous)
>
>
>> +needed and has different nature than time API has. Usually, timers
>> require a
>>
>
> rephrase (simplify): ...and is different than the time API.
> Delete next sentence ("Usually, timers require...") as that's not relevant
> to the API or its usage.
>
>
>> +separate h/w module to be used for generating timeouts. Timers are drawn
>> from
>> +specialized pools called timer pools that have their own abstract type
>> +(+odp_timer_pool_t+). Applications may have many timers active at the
>> same
>> +time and can set them to use either relative or absolute time. When
>> timers
>> +expire they create events of type +odp_timeout_t+, which serve as
>> notifications
>> +of timer expiration. The event is placed on a queue pointed while timer
>>
>
> rephrase (clarify): "...placed on a queue specified at timer allocation."
>
>
>> +allocation.
>> +
>> +Each timer pool can be set with it's own resolution in ns and number of
>>
>
> grammar: its, not it's
>
>
>> +supported timers. So, timer pool can be considered as a time source with
>>
>
> grammar: So, a timer pool can be considered a time source...
>
>
>> +it's own resolution and defined number of timers. All timers in timer
>> pool
>> +are handled with same time source with same resolution. If user needs
>> two types
>> +of timers with different requirements for resolution then better to
>> create
>> +two pools with it's own resolution, it can decrease load on hardware.
>>
>
> grammar and rephrase: All timers in a timer pool have the same time source
> and resolution.
> If two types of timers with different resolutions are needed, then
> applications should create two
> timer pools, each with its own resolution.
>
>
>> +
>> +An expiration time for the timer is set in it's own ticks, so
>> nanoseconds have
>> +to be converted first with conversation function
>> +odp_timer_ns_to_tick()+, to
>> +convert it back to ns use +odp_timer_tick_to_ns()+. Both functions
>> require
>> +to pass a timer pool used, as it can be sourced with it's own time
>> source that
>> +can have specific resolution and thus different conversion ratio.
>>
>
> grammar and rephrase for clarity: Each timer pool has its own
> implementation-dependent
> tick granularity. For portability applications must use the functions
> +odp_timer_ns_to_tick()+
> and +odp_timer_tick_to_ns()+ to convert between ns and the pool's native
> tick values. Note that
> these functions take a timer pool as one of their arguments since each
> pool may have its own
> time source and conversion ratio.
>
>
>> +
>> +To set a timer to deliver a timeout event the two functions can be used:
>> ++odp_timer_set_abs()+ and +odp_timer_set_rel()+. Both of them require an
>> +event to be passed and a time interval in ticks of corresponding timer
>> pool.
>>
>
> grammar: "...a timeout event two functions can..." (delete the)
> " Both of these...."
> " ...in ticks of the corresponding timer pool"
>
>
>> +The expiration time for a timer can be set based on current tick value
>> for
>>
>
> grammar: "...based on the current tick value..."
>
>
>> +a timer pool taken with +odp_timer_current_tick()+, to set a timer
>>
>
> grammar (new sentence): "To set a timer..."
>
>
>> +(absolute time) with a user-provided timeout event, the
>> +odp_timer_set_abs()+
>>
>
> grammar: "...event, +odp_timer_set_abs()+ can be used." (delete the)
>
>
>> +can be used. An event is the event converted from timeout allocated from
>> timeout
>> +pool with +odp_timeout_alloc()+. The event is returned to timer queue
>> when set
>> +time interval has expired and can be also converted to timeout with
>> ++odp_timeout_from_event()+, if it's needed. To free timeout the
>> ++odp_timeout_free()+ can be used, if it's presented as event it can be
>> freed
>> +as event with +odp_event_free()+.
>>
>
> This is fairly confusing and is where the state diagrams are really
> helpful. I'm not going to suggest a wording here but see if doing this in
> the context of the state diagram makes it simpler.
>
>
>> +
>> +In general, timer pool characterizes time source, and timer is
>> characterized
>> +with timer pool, user pointer and destination queue for delivering. The
>> timeout
>> +is characterized with timer, timer pool and time of expiration and is
>> placed to
>> +queue set while timer allocation. The timeout can be delivered only to
>> the
>> +destination queue of concrete timer. To get a timer generated a timeout
>> ++odp_timeout_timer()+ can be used.
>>
>
> This is also fairly muddy as it's trying to connect too many concepts in a
> couple of sentences. I think it's better to describe timers and timeouts
> and how they relate and then walk through the state diagram discussing how
> applications create, set, cancel, and process timer events.
>
>
>> +
>> +When timer is freed it's returned to timer pool and is ready to be
>> allocated
>> +once again. A timer can be canceled with odp_timer_cancel.
>>
>
> use +odp_timer_cancel()+ here for consistency.
>
>
>> === Synchronizer
>> Multiple threads operating in parallel typically require various
>> --
>> 1.9.1
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> https://lists.linaro.org/mailman/listinfo/lng-odp
>>
>
>
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
