diff mbox

[PATCHv2,1/2] doc: userguide: add timer/timeout state diagram

Message ID 1462637708-1491-1-git-send-email-bill.fischofer@linaro.org
State New
Headers show

Commit Message

Bill Fischofer May 7, 2016, 4:15 p.m. UTC
Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>
---
 doc/images/.gitignore       |  1 +
 doc/images/timer_fsm.gv     | 38 ++++++++++++++++++++++++++++++++++++++
 doc/users-guide/Makefile.am |  1 +
 3 files changed, 40 insertions(+)
 create mode 100644 doc/images/timer_fsm.gv

Comments

Christophe Milard May 9, 2016, 12:58 p.m. UTC | #1
I am a bit confused by this diagram: It feels to me that timers and timeout
have separate lives (even , if of course some dependency exist).
From this diagram, it feels like these 2 separate objects (timers and time
out events) share states...? do they?
In my head, and from the understanding I had, it feels that the to 4 top
states are for timers, whereas time-out events have only 2 states:
Unallocated or allocated.
It feels you are doing 1 FSM out of two. Maybe , it should be two FSM
(keeping the same transition names to show the dependancy.)

And I guess there is a typo at "odp_timeout_freee().

Christophe

On 7 May 2016 at 18:15, Bill Fischofer <bill.fischofer@linaro.org> wrote:

> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

> ---

>  doc/images/.gitignore       |  1 +

>  doc/images/timer_fsm.gv     | 38 ++++++++++++++++++++++++++++++++++++++

>  doc/users-guide/Makefile.am |  1 +

>  3 files changed, 40 insertions(+)

>  create mode 100644 doc/images/timer_fsm.gv

>

> diff --git a/doc/images/.gitignore b/doc/images/.gitignore

> index a19aa75..72cf7ec 100644

> --- a/doc/images/.gitignore

> +++ b/doc/images/.gitignore

> @@ -1,2 +1,3 @@

>  resource_management.svg

>  pktio_fsm.svg

> +timer_fsm.svg

> diff --git a/doc/images/timer_fsm.gv b/doc/images/timer_fsm.gv

> new file mode 100644

> index 0000000..f8cb21a

> --- /dev/null

> +++ b/doc/images/timer_fsm.gv

> @@ -0,0 +1,38 @@

> +digraph timer_state_machine {

> +       rankdir=LR;

> +       size="12,20";

> +       node [fontsize=28];

> +       edge [fontsize=28];

> +       node [shape=doublecircle]; Timer_Unalloc

> +                                  Timeout_Unalloc

> +                                  Timeout_Delivered;

> +        node [shape=rectangle]; Timeout_Queued;

> +       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 -> Timeout_Queued [label="=>odp_queue_enq()"];

> +       Timeout_Queued -> 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];

> +}

> diff --git a/doc/users-guide/Makefile.am b/doc/users-guide/Makefile.am

> index 74caa96..6bb0131 100644

> --- a/doc/users-guide/Makefile.am

> +++ b/doc/users-guide/Makefile.am

> @@ -30,6 +30,7 @@ IMAGES = $(top_srcdir)/doc/images/overview.svg \

>          $(top_srcdir)/doc/images/release_git.svg \

>          $(top_srcdir)/doc/images/segment.svg \

>          $(top_srcdir)/doc/images/simple_release_git.svg \

> +        $(top_srcdir)/doc/images/timer_fsm.svg \

>          $(top_srcdir)/doc/images/tm_hierarchy.svg \

>          $(top_srcdir)/doc/images/tm_node.svg

>

> --

> 2.5.0

>

> _______________________________________________

> lng-odp mailing list

> lng-odp@lists.linaro.org

> https://lists.linaro.org/mailman/listinfo/lng-odp

>
Bill Fischofer May 9, 2016, 2:28 p.m. UTC | #2
On Mon, May 9, 2016 at 7:58 AM, Christophe Milard <
christophe.milard@linaro.org> wrote:

> I am a bit confused by this diagram: It feels to me that timers and

> timeout have separate lives (even , if of course some dependency exist).

> From this diagram, it feels like these 2 separate objects (timers and time

> out events) share states...? do they?

>


Actually they do, which is what this diagram is trying to express. When a
timer is set one of the arguments is the timeout event that should be
associated with it, so it is an error to attempt to free that event while
it is associated with a set timer (results are undefined if you do).
Timers are somewhat unique in this respect.


> In my head, and from the understanding I had, it feels that the to 4 top

> states are for timers, whereas time-out events have only 2 states:

> Unallocated or allocated.

> It feels you are doing 1 FSM out of two. Maybe , it should be two FSM

> (keeping the same transition names to show the dependancy.)

>

> And I guess there is a typo at "odp_timeout_freee().

>


Thanks.  I'll fix that in the next version.


>

> Christophe

>

> On 7 May 2016 at 18:15, Bill Fischofer <bill.fischofer@linaro.org> wrote:

>

>> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

>> ---

>>  doc/images/.gitignore       |  1 +

>>  doc/images/timer_fsm.gv     | 38 ++++++++++++++++++++++++++++++++++++++

>>  doc/users-guide/Makefile.am |  1 +

>>  3 files changed, 40 insertions(+)

>>  create mode 100644 doc/images/timer_fsm.gv

>>

>> diff --git a/doc/images/.gitignore b/doc/images/.gitignore

>> index a19aa75..72cf7ec 100644

>> --- a/doc/images/.gitignore

>> +++ b/doc/images/.gitignore

>> @@ -1,2 +1,3 @@

>>  resource_management.svg

>>  pktio_fsm.svg

>> +timer_fsm.svg

>> diff --git a/doc/images/timer_fsm.gv b/doc/images/timer_fsm.gv

>> new file mode 100644

>> index 0000000..f8cb21a

>> --- /dev/null

>> +++ b/doc/images/timer_fsm.gv

>> @@ -0,0 +1,38 @@

>> +digraph timer_state_machine {

>> +       rankdir=LR;

>> +       size="12,20";

>> +       node [fontsize=28];

>> +       edge [fontsize=28];

>> +       node [shape=doublecircle]; Timer_Unalloc

>> +                                  Timeout_Unalloc

>> +                                  Timeout_Delivered;

>> +        node [shape=rectangle]; Timeout_Queued;

>> +       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 -> Timeout_Queued [label="=>odp_queue_enq()"];

>> +       Timeout_Queued -> 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];

>> +}

>> diff --git a/doc/users-guide/Makefile.am b/doc/users-guide/Makefile.am

>> index 74caa96..6bb0131 100644

>> --- a/doc/users-guide/Makefile.am

>> +++ b/doc/users-guide/Makefile.am

>> @@ -30,6 +30,7 @@ IMAGES = $(top_srcdir)/doc/images/overview.svg \

>>          $(top_srcdir)/doc/images/release_git.svg \

>>          $(top_srcdir)/doc/images/segment.svg \

>>          $(top_srcdir)/doc/images/simple_release_git.svg \

>> +        $(top_srcdir)/doc/images/timer_fsm.svg \

>>          $(top_srcdir)/doc/images/tm_hierarchy.svg \

>>          $(top_srcdir)/doc/images/tm_node.svg

>>

>> --

>> 2.5.0

>>

>> _______________________________________________

>> lng-odp mailing list

>> lng-odp@lists.linaro.org

>> https://lists.linaro.org/mailman/listinfo/lng-odp

>>

>

>
Christophe Milard May 9, 2016, 2:39 p.m. UTC | #3
On 9 May 2016 at 16:28, Bill Fischofer <bill.fischofer@linaro.org> wrote:

>

>

> On Mon, May 9, 2016 at 7:58 AM, Christophe Milard <

> christophe.milard@linaro.org> wrote:

>

>> I am a bit confused by this diagram: It feels to me that timers and

>> timeout have separate lives (even , if of course some dependency exist).

>> From this diagram, it feels like these 2 separate objects (timers and

>> time out events) share states...? do they?

>>

>

> Actually they do, which is what this diagram is trying to express. When a

> timer is set one of the arguments is the timeout event that should be

> associated with it, so it is an error to attempt to free that event while

> it is associated with a set timer (results are undefined if you do).

> Timers are somewhat unique in this respect.

>


Sorry, Bill I still don't get it: Aren't you saying here that these are 2
separate FSMs, but that these 2 separate FSMs are actually
actionned/"triggered" by (partly) the same events? There is nothing wrong
with that... Then they should be represented as 2 separated FSM with the
same event names on the transitions triggered by the same events...
 Or I am very confused...

Christophe.

>

>

>> In my head, and from the understanding I had, it feels that the to 4 top

>> states are for timers, whereas time-out events have only 2 states:

>> Unallocated or allocated.

>> It feels you are doing 1 FSM out of two. Maybe , it should be two FSM

>> (keeping the same transition names to show the dependancy.)

>>

>> And I guess there is a typo at "odp_timeout_freee().

>>

>

> Thanks.  I'll fix that in the next version.

>

>

>>

>> Christophe

>>

>> On 7 May 2016 at 18:15, Bill Fischofer <bill.fischofer@linaro.org> wrote:

>>

>>> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

>>> ---

>>>  doc/images/.gitignore       |  1 +

>>>  doc/images/timer_fsm.gv     | 38 ++++++++++++++++++++++++++++++++++++++

>>>  doc/users-guide/Makefile.am |  1 +

>>>  3 files changed, 40 insertions(+)

>>>  create mode 100644 doc/images/timer_fsm.gv

>>>

>>> diff --git a/doc/images/.gitignore b/doc/images/.gitignore

>>> index a19aa75..72cf7ec 100644

>>> --- a/doc/images/.gitignore

>>> +++ b/doc/images/.gitignore

>>> @@ -1,2 +1,3 @@

>>>  resource_management.svg

>>>  pktio_fsm.svg

>>> +timer_fsm.svg

>>> diff --git a/doc/images/timer_fsm.gv b/doc/images/timer_fsm.gv

>>> new file mode 100644

>>> index 0000000..f8cb21a

>>> --- /dev/null

>>> +++ b/doc/images/timer_fsm.gv

>>> @@ -0,0 +1,38 @@

>>> +digraph timer_state_machine {

>>> +       rankdir=LR;

>>> +       size="12,20";

>>> +       node [fontsize=28];

>>> +       edge [fontsize=28];

>>> +       node [shape=doublecircle]; Timer_Unalloc

>>> +                                  Timeout_Unalloc

>>> +                                  Timeout_Delivered;

>>> +        node [shape=rectangle]; Timeout_Queued;

>>> +       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 -> Timeout_Queued [label="=>odp_queue_enq()"];

>>> +       Timeout_Queued -> 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];

>>> +}

>>> diff --git a/doc/users-guide/Makefile.am b/doc/users-guide/Makefile.am

>>> index 74caa96..6bb0131 100644

>>> --- a/doc/users-guide/Makefile.am

>>> +++ b/doc/users-guide/Makefile.am

>>> @@ -30,6 +30,7 @@ IMAGES = $(top_srcdir)/doc/images/overview.svg \

>>>          $(top_srcdir)/doc/images/release_git.svg \

>>>          $(top_srcdir)/doc/images/segment.svg \

>>>          $(top_srcdir)/doc/images/simple_release_git.svg \

>>> +        $(top_srcdir)/doc/images/timer_fsm.svg \

>>>          $(top_srcdir)/doc/images/tm_hierarchy.svg \

>>>          $(top_srcdir)/doc/images/tm_node.svg

>>>

>>> --

>>> 2.5.0

>>>

>>> _______________________________________________

>>> lng-odp mailing list

>>> lng-odp@lists.linaro.org

>>> https://lists.linaro.org/mailman/listinfo/lng-odp

>>>

>>

>>

>
Bill Fischofer May 9, 2016, 3:15 p.m. UTC | #4
As we discussed in our call, we can both play around with graphviz and see
if a prettier diagram can be constructed that is perhaps clearer, possibly
with two separate FSMs.

On Mon, May 9, 2016 at 9:39 AM, Christophe Milard <
christophe.milard@linaro.org> wrote:

>

>

> On 9 May 2016 at 16:28, Bill Fischofer <bill.fischofer@linaro.org> wrote:

>

>>

>>

>> On Mon, May 9, 2016 at 7:58 AM, Christophe Milard <

>> christophe.milard@linaro.org> wrote:

>>

>>> I am a bit confused by this diagram: It feels to me that timers and

>>> timeout have separate lives (even , if of course some dependency exist).

>>> From this diagram, it feels like these 2 separate objects (timers and

>>> time out events) share states...? do they?

>>>

>>

>> Actually they do, which is what this diagram is trying to express. When a

>> timer is set one of the arguments is the timeout event that should be

>> associated with it, so it is an error to attempt to free that event while

>> it is associated with a set timer (results are undefined if you do).

>> Timers are somewhat unique in this respect.

>>

>

> Sorry, Bill I still don't get it: Aren't you saying here that these are 2

> separate FSMs, but that these 2 separate FSMs are actually

> actionned/"triggered" by (partly) the same events? There is nothing wrong

> with that... Then they should be represented as 2 separated FSM with the

> same event names on the transitions triggered by the same events...

>  Or I am very confused...

>

> Christophe.

>

>>

>>

>>> In my head, and from the understanding I had, it feels that the to 4 top

>>> states are for timers, whereas time-out events have only 2 states:

>>> Unallocated or allocated.

>>> It feels you are doing 1 FSM out of two. Maybe , it should be two FSM

>>> (keeping the same transition names to show the dependancy.)

>>>

>>> And I guess there is a typo at "odp_timeout_freee().

>>>

>>

>> Thanks.  I'll fix that in the next version.

>>

>>

>>>

>>> Christophe

>>>

>>> On 7 May 2016 at 18:15, Bill Fischofer <bill.fischofer@linaro.org>

>>> wrote:

>>>

>>>> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

>>>> ---

>>>>  doc/images/.gitignore       |  1 +

>>>>  doc/images/timer_fsm.gv     | 38 ++++++++++++++++++++++++++++++++++++++

>>>>  doc/users-guide/Makefile.am |  1 +

>>>>  3 files changed, 40 insertions(+)

>>>>  create mode 100644 doc/images/timer_fsm.gv

>>>>

>>>> diff --git a/doc/images/.gitignore b/doc/images/.gitignore

>>>> index a19aa75..72cf7ec 100644

>>>> --- a/doc/images/.gitignore

>>>> +++ b/doc/images/.gitignore

>>>> @@ -1,2 +1,3 @@

>>>>  resource_management.svg

>>>>  pktio_fsm.svg

>>>> +timer_fsm.svg

>>>> diff --git a/doc/images/timer_fsm.gv b/doc/images/timer_fsm.gv

>>>> new file mode 100644

>>>> index 0000000..f8cb21a

>>>> --- /dev/null

>>>> +++ b/doc/images/timer_fsm.gv

>>>> @@ -0,0 +1,38 @@

>>>> +digraph timer_state_machine {

>>>> +       rankdir=LR;

>>>> +       size="12,20";

>>>> +       node [fontsize=28];

>>>> +       edge [fontsize=28];

>>>> +       node [shape=doublecircle]; Timer_Unalloc

>>>> +                                  Timeout_Unalloc

>>>> +                                  Timeout_Delivered;

>>>> +        node [shape=rectangle]; Timeout_Queued;

>>>> +       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 -> Timeout_Queued [label="=>odp_queue_enq()"];

>>>> +       Timeout_Queued -> 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];

>>>> +}

>>>> diff --git a/doc/users-guide/Makefile.am b/doc/users-guide/Makefile.am

>>>> index 74caa96..6bb0131 100644

>>>> --- a/doc/users-guide/Makefile.am

>>>> +++ b/doc/users-guide/Makefile.am

>>>> @@ -30,6 +30,7 @@ IMAGES = $(top_srcdir)/doc/images/overview.svg \

>>>>          $(top_srcdir)/doc/images/release_git.svg \

>>>>          $(top_srcdir)/doc/images/segment.svg \

>>>>          $(top_srcdir)/doc/images/simple_release_git.svg \

>>>> +        $(top_srcdir)/doc/images/timer_fsm.svg \

>>>>          $(top_srcdir)/doc/images/tm_hierarchy.svg \

>>>>          $(top_srcdir)/doc/images/tm_node.svg

>>>>

>>>> --

>>>> 2.5.0

>>>>

>>>> _______________________________________________

>>>> lng-odp mailing list

>>>> lng-odp@lists.linaro.org

>>>> https://lists.linaro.org/mailman/listinfo/lng-odp

>>>>

>>>

>>>

>>

>
Christophe Milard May 10, 2016, 8:45 a.m. UTC | #5
Hi Bill,

Just sent you an RFC with 2 FSMs.
Had to fight a bit with graphviz. Not finished yet. I will continue working
on it if you think that makes sense...
Christophe.

On 9 May 2016 at 17:15, Bill Fischofer <bill.fischofer@linaro.org> wrote:

> As we discussed in our call, we can both play around with graphviz and see

> if a prettier diagram can be constructed that is perhaps clearer, possibly

> with two separate FSMs.

>

> On Mon, May 9, 2016 at 9:39 AM, Christophe Milard <

> christophe.milard@linaro.org> wrote:

>

>>

>>

>> On 9 May 2016 at 16:28, Bill Fischofer <bill.fischofer@linaro.org> wrote:

>>

>>>

>>>

>>> On Mon, May 9, 2016 at 7:58 AM, Christophe Milard <

>>> christophe.milard@linaro.org> wrote:

>>>

>>>> I am a bit confused by this diagram: It feels to me that timers and

>>>> timeout have separate lives (even , if of course some dependency exist).

>>>> From this diagram, it feels like these 2 separate objects (timers and

>>>> time out events) share states...? do they?

>>>>

>>>

>>> Actually they do, which is what this diagram is trying to express. When

>>> a timer is set one of the arguments is the timeout event that should be

>>> associated with it, so it is an error to attempt to free that event while

>>> it is associated with a set timer (results are undefined if you do).

>>> Timers are somewhat unique in this respect.

>>>

>>

>> Sorry, Bill I still don't get it: Aren't you saying here that these are 2

>> separate FSMs, but that these 2 separate FSMs are actually

>> actionned/"triggered" by (partly) the same events? There is nothing wrong

>> with that... Then they should be represented as 2 separated FSM with the

>> same event names on the transitions triggered by the same events...

>>  Or I am very confused...

>>

>> Christophe.

>>

>>>

>>>

>>>> In my head, and from the understanding I had, it feels that the to 4

>>>> top states are for timers, whereas time-out events have only 2 states:

>>>> Unallocated or allocated.

>>>> It feels you are doing 1 FSM out of two. Maybe , it should be two FSM

>>>> (keeping the same transition names to show the dependancy.)

>>>>

>>>> And I guess there is a typo at "odp_timeout_freee().

>>>>

>>>

>>> Thanks.  I'll fix that in the next version.

>>>

>>>

>>>>

>>>> Christophe

>>>>

>>>> On 7 May 2016 at 18:15, Bill Fischofer <bill.fischofer@linaro.org>

>>>> wrote:

>>>>

>>>>> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

>>>>> ---

>>>>>  doc/images/.gitignore       |  1 +

>>>>>  doc/images/timer_fsm.gv     | 38

>>>>> ++++++++++++++++++++++++++++++++++++++

>>>>>  doc/users-guide/Makefile.am |  1 +

>>>>>  3 files changed, 40 insertions(+)

>>>>>  create mode 100644 doc/images/timer_fsm.gv

>>>>>

>>>>> diff --git a/doc/images/.gitignore b/doc/images/.gitignore

>>>>> index a19aa75..72cf7ec 100644

>>>>> --- a/doc/images/.gitignore

>>>>> +++ b/doc/images/.gitignore

>>>>> @@ -1,2 +1,3 @@

>>>>>  resource_management.svg

>>>>>  pktio_fsm.svg

>>>>> +timer_fsm.svg

>>>>> diff --git a/doc/images/timer_fsm.gv b/doc/images/timer_fsm.gv

>>>>> new file mode 100644

>>>>> index 0000000..f8cb21a

>>>>> --- /dev/null

>>>>> +++ b/doc/images/timer_fsm.gv

>>>>> @@ -0,0 +1,38 @@

>>>>> +digraph timer_state_machine {

>>>>> +       rankdir=LR;

>>>>> +       size="12,20";

>>>>> +       node [fontsize=28];

>>>>> +       edge [fontsize=28];

>>>>> +       node [shape=doublecircle]; Timer_Unalloc

>>>>> +                                  Timeout_Unalloc

>>>>> +                                  Timeout_Delivered;

>>>>> +        node [shape=rectangle]; Timeout_Queued;

>>>>> +       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 -> Timeout_Queued [label="=>odp_queue_enq()"];

>>>>> +       Timeout_Queued -> 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];

>>>>> +}

>>>>> diff --git a/doc/users-guide/Makefile.am b/doc/users-guide/Makefile.am

>>>>> index 74caa96..6bb0131 100644

>>>>> --- a/doc/users-guide/Makefile.am

>>>>> +++ b/doc/users-guide/Makefile.am

>>>>> @@ -30,6 +30,7 @@ IMAGES = $(top_srcdir)/doc/images/overview.svg \

>>>>>          $(top_srcdir)/doc/images/release_git.svg \

>>>>>          $(top_srcdir)/doc/images/segment.svg \

>>>>>          $(top_srcdir)/doc/images/simple_release_git.svg \

>>>>> +        $(top_srcdir)/doc/images/timer_fsm.svg \

>>>>>          $(top_srcdir)/doc/images/tm_hierarchy.svg \

>>>>>          $(top_srcdir)/doc/images/tm_node.svg

>>>>>

>>>>> --

>>>>> 2.5.0

>>>>>

>>>>> _______________________________________________

>>>>> lng-odp mailing list

>>>>> lng-odp@lists.linaro.org

>>>>> https://lists.linaro.org/mailman/listinfo/lng-odp

>>>>>

>>>>

>>>>

>>>

>>

>
Bill Fischofer May 10, 2016, 11:47 p.m. UTC | #6
I looked at them and I liked the added color so reworked my diagram adding
that and posted a v3 for it. I'm not sure trying to separate into two
diagrams really helps but perhaps this reworked diagram is easier to follow?

On Tue, May 10, 2016 at 3:45 AM, Christophe Milard <
christophe.milard@linaro.org> wrote:

> Hi Bill,

>

> Just sent you an RFC with 2 FSMs.

> Had to fight a bit with graphviz. Not finished yet. I will continue

> working on it if you think that makes sense...

> Christophe.

>

> On 9 May 2016 at 17:15, Bill Fischofer <bill.fischofer@linaro.org> wrote:

>

>> As we discussed in our call, we can both play around with graphviz and

>> see if a prettier diagram can be constructed that is perhaps clearer,

>> possibly with two separate FSMs.

>>

>> On Mon, May 9, 2016 at 9:39 AM, Christophe Milard <

>> christophe.milard@linaro.org> wrote:

>>

>>>

>>>

>>> On 9 May 2016 at 16:28, Bill Fischofer <bill.fischofer@linaro.org>

>>> wrote:

>>>

>>>>

>>>>

>>>> On Mon, May 9, 2016 at 7:58 AM, Christophe Milard <

>>>> christophe.milard@linaro.org> wrote:

>>>>

>>>>> I am a bit confused by this diagram: It feels to me that timers and

>>>>> timeout have separate lives (even , if of course some dependency exist).

>>>>> From this diagram, it feels like these 2 separate objects (timers and

>>>>> time out events) share states...? do they?

>>>>>

>>>>

>>>> Actually they do, which is what this diagram is trying to express. When

>>>> a timer is set one of the arguments is the timeout event that should be

>>>> associated with it, so it is an error to attempt to free that event while

>>>> it is associated with a set timer (results are undefined if you do).

>>>> Timers are somewhat unique in this respect.

>>>>

>>>

>>> Sorry, Bill I still don't get it: Aren't you saying here that these are

>>> 2 separate FSMs, but that these 2 separate FSMs are actually

>>> actionned/"triggered" by (partly) the same events? There is nothing wrong

>>> with that... Then they should be represented as 2 separated FSM with the

>>> same event names on the transitions triggered by the same events...

>>>  Or I am very confused...

>>>

>>> Christophe.

>>>

>>>>

>>>>

>>>>> In my head, and from the understanding I had, it feels that the to 4

>>>>> top states are for timers, whereas time-out events have only 2 states:

>>>>> Unallocated or allocated.

>>>>> It feels you are doing 1 FSM out of two. Maybe , it should be two FSM

>>>>> (keeping the same transition names to show the dependancy.)

>>>>>

>>>>> And I guess there is a typo at "odp_timeout_freee().

>>>>>

>>>>

>>>> Thanks.  I'll fix that in the next version.

>>>>

>>>>

>>>>>

>>>>> Christophe

>>>>>

>>>>> On 7 May 2016 at 18:15, Bill Fischofer <bill.fischofer@linaro.org>

>>>>> wrote:

>>>>>

>>>>>> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

>>>>>> ---

>>>>>>  doc/images/.gitignore       |  1 +

>>>>>>  doc/images/timer_fsm.gv     | 38

>>>>>> ++++++++++++++++++++++++++++++++++++++

>>>>>>  doc/users-guide/Makefile.am |  1 +

>>>>>>  3 files changed, 40 insertions(+)

>>>>>>  create mode 100644 doc/images/timer_fsm.gv

>>>>>>

>>>>>> diff --git a/doc/images/.gitignore b/doc/images/.gitignore

>>>>>> index a19aa75..72cf7ec 100644

>>>>>> --- a/doc/images/.gitignore

>>>>>> +++ b/doc/images/.gitignore

>>>>>> @@ -1,2 +1,3 @@

>>>>>>  resource_management.svg

>>>>>>  pktio_fsm.svg

>>>>>> +timer_fsm.svg

>>>>>> diff --git a/doc/images/timer_fsm.gv b/doc/images/timer_fsm.gv

>>>>>> new file mode 100644

>>>>>> index 0000000..f8cb21a

>>>>>> --- /dev/null

>>>>>> +++ b/doc/images/timer_fsm.gv

>>>>>> @@ -0,0 +1,38 @@

>>>>>> +digraph timer_state_machine {

>>>>>> +       rankdir=LR;

>>>>>> +       size="12,20";

>>>>>> +       node [fontsize=28];

>>>>>> +       edge [fontsize=28];

>>>>>> +       node [shape=doublecircle]; Timer_Unalloc

>>>>>> +                                  Timeout_Unalloc

>>>>>> +                                  Timeout_Delivered;

>>>>>> +        node [shape=rectangle]; Timeout_Queued;

>>>>>> +       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 -> Timeout_Queued [label="=>odp_queue_enq()"];

>>>>>> +       Timeout_Queued -> 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];

>>>>>> +}

>>>>>> diff --git a/doc/users-guide/Makefile.am b/doc/users-guide/Makefile.am

>>>>>> index 74caa96..6bb0131 100644

>>>>>> --- a/doc/users-guide/Makefile.am

>>>>>> +++ b/doc/users-guide/Makefile.am

>>>>>> @@ -30,6 +30,7 @@ IMAGES = $(top_srcdir)/doc/images/overview.svg \

>>>>>>          $(top_srcdir)/doc/images/release_git.svg \

>>>>>>          $(top_srcdir)/doc/images/segment.svg \

>>>>>>          $(top_srcdir)/doc/images/simple_release_git.svg \

>>>>>> +        $(top_srcdir)/doc/images/timer_fsm.svg \

>>>>>>          $(top_srcdir)/doc/images/tm_hierarchy.svg \

>>>>>>          $(top_srcdir)/doc/images/tm_node.svg

>>>>>>

>>>>>> --

>>>>>> 2.5.0

>>>>>>

>>>>>> _______________________________________________

>>>>>> lng-odp mailing list

>>>>>> lng-odp@lists.linaro.org

>>>>>> https://lists.linaro.org/mailman/listinfo/lng-odp

>>>>>>

>>>>>

>>>>>

>>>>

>>>

>>

>
Christophe Milard May 12, 2016, 8:13 a.m. UTC | #7
Hi,

I am still confused with this shared FSM for the 2 distinct objects: The
red seems to be actions associated with the transitions (and the timer
expiration event is no longer shown on the transition showing the actions).
I am not sure what the distinction between the green and blue events are.
I propose the following: I will send a complete proposal to you, including
both the FSMs and the text. If we still disagree on which one is clearer,
we'll let other reviewer decide (you/I would sent a v4 with my stuff in).
If no one does react, I am ok to mark your proposal as reviewed because
some doc is better than none.

So you are sure to  win :-)

Christophe.

On 11 May 2016 at 01:47, Bill Fischofer <bill.fischofer@linaro.org> wrote:

> I looked at them and I liked the added color so reworked my diagram adding

> that and posted a v3 for it. I'm not sure trying to separate into two

> diagrams really helps but perhaps this reworked diagram is easier to follow?

>

> On Tue, May 10, 2016 at 3:45 AM, Christophe Milard <

> christophe.milard@linaro.org> wrote:

>

>> Hi Bill,

>>

>> Just sent you an RFC with 2 FSMs.

>> Had to fight a bit with graphviz. Not finished yet. I will continue

>> working on it if you think that makes sense...

>> Christophe.

>>

>> On 9 May 2016 at 17:15, Bill Fischofer <bill.fischofer@linaro.org> wrote:

>>

>>> As we discussed in our call, we can both play around with graphviz and

>>> see if a prettier diagram can be constructed that is perhaps clearer,

>>> possibly with two separate FSMs.

>>>

>>> On Mon, May 9, 2016 at 9:39 AM, Christophe Milard <

>>> christophe.milard@linaro.org> wrote:

>>>

>>>>

>>>>

>>>> On 9 May 2016 at 16:28, Bill Fischofer <bill.fischofer@linaro.org>

>>>> wrote:

>>>>

>>>>>

>>>>>

>>>>> On Mon, May 9, 2016 at 7:58 AM, Christophe Milard <

>>>>> christophe.milard@linaro.org> wrote:

>>>>>

>>>>>> I am a bit confused by this diagram: It feels to me that timers and

>>>>>> timeout have separate lives (even , if of course some dependency exist).

>>>>>> From this diagram, it feels like these 2 separate objects (timers and

>>>>>> time out events) share states...? do they?

>>>>>>

>>>>>

>>>>> Actually they do, which is what this diagram is trying to express.

>>>>> When a timer is set one of the arguments is the timeout event that should

>>>>> be associated with it, so it is an error to attempt to free that event

>>>>> while it is associated with a set timer (results are undefined if you do).

>>>>> Timers are somewhat unique in this respect.

>>>>>

>>>>

>>>> Sorry, Bill I still don't get it: Aren't you saying here that these are

>>>> 2 separate FSMs, but that these 2 separate FSMs are actually

>>>> actionned/"triggered" by (partly) the same events? There is nothing wrong

>>>> with that... Then they should be represented as 2 separated FSM with the

>>>> same event names on the transitions triggered by the same events...

>>>>  Or I am very confused...

>>>>

>>>> Christophe.

>>>>

>>>>>

>>>>>

>>>>>> In my head, and from the understanding I had, it feels that the to 4

>>>>>> top states are for timers, whereas time-out events have only 2 states:

>>>>>> Unallocated or allocated.

>>>>>> It feels you are doing 1 FSM out of two. Maybe , it should be two FSM

>>>>>> (keeping the same transition names to show the dependancy.)

>>>>>>

>>>>>> And I guess there is a typo at "odp_timeout_freee().

>>>>>>

>>>>>

>>>>> Thanks.  I'll fix that in the next version.

>>>>>

>>>>>

>>>>>>

>>>>>> Christophe

>>>>>>

>>>>>> On 7 May 2016 at 18:15, Bill Fischofer <bill.fischofer@linaro.org>

>>>>>> wrote:

>>>>>>

>>>>>>> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

>>>>>>> ---

>>>>>>>  doc/images/.gitignore       |  1 +

>>>>>>>  doc/images/timer_fsm.gv     | 38

>>>>>>> ++++++++++++++++++++++++++++++++++++++

>>>>>>>  doc/users-guide/Makefile.am |  1 +

>>>>>>>  3 files changed, 40 insertions(+)

>>>>>>>  create mode 100644 doc/images/timer_fsm.gv

>>>>>>>

>>>>>>> diff --git a/doc/images/.gitignore b/doc/images/.gitignore

>>>>>>> index a19aa75..72cf7ec 100644

>>>>>>> --- a/doc/images/.gitignore

>>>>>>> +++ b/doc/images/.gitignore

>>>>>>> @@ -1,2 +1,3 @@

>>>>>>>  resource_management.svg

>>>>>>>  pktio_fsm.svg

>>>>>>> +timer_fsm.svg

>>>>>>> diff --git a/doc/images/timer_fsm.gv b/doc/images/timer_fsm.gv

>>>>>>> new file mode 100644

>>>>>>> index 0000000..f8cb21a

>>>>>>> --- /dev/null

>>>>>>> +++ b/doc/images/timer_fsm.gv

>>>>>>> @@ -0,0 +1,38 @@

>>>>>>> +digraph timer_state_machine {

>>>>>>> +       rankdir=LR;

>>>>>>> +       size="12,20";

>>>>>>> +       node [fontsize=28];

>>>>>>> +       edge [fontsize=28];

>>>>>>> +       node [shape=doublecircle]; Timer_Unalloc

>>>>>>> +                                  Timeout_Unalloc

>>>>>>> +                                  Timeout_Delivered;

>>>>>>> +        node [shape=rectangle]; Timeout_Queued;

>>>>>>> +       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 -> Timeout_Queued [label="=>odp_queue_enq()"];

>>>>>>> +       Timeout_Queued -> 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];

>>>>>>> +}

>>>>>>> diff --git a/doc/users-guide/Makefile.am

>>>>>>> b/doc/users-guide/Makefile.am

>>>>>>> index 74caa96..6bb0131 100644

>>>>>>> --- a/doc/users-guide/Makefile.am

>>>>>>> +++ b/doc/users-guide/Makefile.am

>>>>>>> @@ -30,6 +30,7 @@ IMAGES = $(top_srcdir)/doc/images/overview.svg \

>>>>>>>          $(top_srcdir)/doc/images/release_git.svg \

>>>>>>>          $(top_srcdir)/doc/images/segment.svg \

>>>>>>>          $(top_srcdir)/doc/images/simple_release_git.svg \

>>>>>>> +        $(top_srcdir)/doc/images/timer_fsm.svg \

>>>>>>>          $(top_srcdir)/doc/images/tm_hierarchy.svg \

>>>>>>>          $(top_srcdir)/doc/images/tm_node.svg

>>>>>>>

>>>>>>> --

>>>>>>> 2.5.0

>>>>>>>

>>>>>>> _______________________________________________

>>>>>>> lng-odp mailing list

>>>>>>> lng-odp@lists.linaro.org

>>>>>>> https://lists.linaro.org/mailman/listinfo/lng-odp

>>>>>>>

>>>>>>

>>>>>>

>>>>>

>>>>

>>>

>>

>
Bill Fischofer May 12, 2016, 12:08 p.m. UTC | #8
The green is for Timer related transitions while the blue is for Timeout
related transitions. This format also seemed to get rid of some of the
annoying line crossings. The red arrows are the transitions that relate to
timer expiration and resulting timeout event scheduling, neither of which
involve Timer APIs. So the colors were supposed to make things easier to
read.

I'm all for improving clarity, so if you have a better way of diagraming
this that's great. I believe the diagrams and text go together in either
case and are intended to be taken together to improve understanding.  The
goal of each of these sections of the User Guide should be to give readers
confidence that they understand how to use these APIs in writing their own
ODP applications, or for ODP implementers to understand the semantics they
need to provide in their own implementations of them.

On Thu, May 12, 2016 at 3:13 AM, Christophe Milard <
christophe.milard@linaro.org> wrote:

> Hi,

>

> I am still confused with this shared FSM for the 2 distinct objects: The

> red seems to be actions associated with the transitions (and the timer

> expiration event is no longer shown on the transition showing the actions).

> I am not sure what the distinction between the green and blue events are.

> I propose the following: I will send a complete proposal to you, including

> both the FSMs and the text. If we still disagree on which one is clearer,

> we'll let other reviewer decide (you/I would sent a v4 with my stuff in).

> If no one does react, I am ok to mark your proposal as reviewed because

> some doc is better than none.

>

> So you are sure to  win :-)

>

> Christophe.

>

> On 11 May 2016 at 01:47, Bill Fischofer <bill.fischofer@linaro.org> wrote:

>

>> I looked at them and I liked the added color so reworked my diagram

>> adding that and posted a v3 for it. I'm not sure trying to separate into

>> two diagrams really helps but perhaps this reworked diagram is easier to

>> follow?

>>

>> On Tue, May 10, 2016 at 3:45 AM, Christophe Milard <

>> christophe.milard@linaro.org> wrote:

>>

>>> Hi Bill,

>>>

>>> Just sent you an RFC with 2 FSMs.

>>> Had to fight a bit with graphviz. Not finished yet. I will continue

>>> working on it if you think that makes sense...

>>> Christophe.

>>>

>>> On 9 May 2016 at 17:15, Bill Fischofer <bill.fischofer@linaro.org>

>>> wrote:

>>>

>>>> As we discussed in our call, we can both play around with graphviz and

>>>> see if a prettier diagram can be constructed that is perhaps clearer,

>>>> possibly with two separate FSMs.

>>>>

>>>> On Mon, May 9, 2016 at 9:39 AM, Christophe Milard <

>>>> christophe.milard@linaro.org> wrote:

>>>>

>>>>>

>>>>>

>>>>> On 9 May 2016 at 16:28, Bill Fischofer <bill.fischofer@linaro.org>

>>>>> wrote:

>>>>>

>>>>>>

>>>>>>

>>>>>> On Mon, May 9, 2016 at 7:58 AM, Christophe Milard <

>>>>>> christophe.milard@linaro.org> wrote:

>>>>>>

>>>>>>> I am a bit confused by this diagram: It feels to me that timers and

>>>>>>> timeout have separate lives (even , if of course some dependency exist).

>>>>>>> From this diagram, it feels like these 2 separate objects (timers

>>>>>>> and time out events) share states...? do they?

>>>>>>>

>>>>>>

>>>>>> Actually they do, which is what this diagram is trying to express.

>>>>>> When a timer is set one of the arguments is the timeout event that should

>>>>>> be associated with it, so it is an error to attempt to free that event

>>>>>> while it is associated with a set timer (results are undefined if you do).

>>>>>> Timers are somewhat unique in this respect.

>>>>>>

>>>>>

>>>>> Sorry, Bill I still don't get it: Aren't you saying here that these

>>>>> are 2 separate FSMs, but that these 2 separate FSMs are actually

>>>>> actionned/"triggered" by (partly) the same events? There is nothing wrong

>>>>> with that... Then they should be represented as 2 separated FSM with the

>>>>> same event names on the transitions triggered by the same events...

>>>>>  Or I am very confused...

>>>>>

>>>>> Christophe.

>>>>>

>>>>>>

>>>>>>

>>>>>>> In my head, and from the understanding I had, it feels that the to 4

>>>>>>> top states are for timers, whereas time-out events have only 2 states:

>>>>>>> Unallocated or allocated.

>>>>>>> It feels you are doing 1 FSM out of two. Maybe , it should be two

>>>>>>> FSM (keeping the same transition names to show the dependancy.)

>>>>>>>

>>>>>>> And I guess there is a typo at "odp_timeout_freee().

>>>>>>>

>>>>>>

>>>>>> Thanks.  I'll fix that in the next version.

>>>>>>

>>>>>>

>>>>>>>

>>>>>>> Christophe

>>>>>>>

>>>>>>> On 7 May 2016 at 18:15, Bill Fischofer <bill.fischofer@linaro.org>

>>>>>>> wrote:

>>>>>>>

>>>>>>>> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

>>>>>>>> ---

>>>>>>>>  doc/images/.gitignore       |  1 +

>>>>>>>>  doc/images/timer_fsm.gv     | 38

>>>>>>>> ++++++++++++++++++++++++++++++++++++++

>>>>>>>>  doc/users-guide/Makefile.am |  1 +

>>>>>>>>  3 files changed, 40 insertions(+)

>>>>>>>>  create mode 100644 doc/images/timer_fsm.gv

>>>>>>>>

>>>>>>>> diff --git a/doc/images/.gitignore b/doc/images/.gitignore

>>>>>>>> index a19aa75..72cf7ec 100644

>>>>>>>> --- a/doc/images/.gitignore

>>>>>>>> +++ b/doc/images/.gitignore

>>>>>>>> @@ -1,2 +1,3 @@

>>>>>>>>  resource_management.svg

>>>>>>>>  pktio_fsm.svg

>>>>>>>> +timer_fsm.svg

>>>>>>>> diff --git a/doc/images/timer_fsm.gv b/doc/images/timer_fsm.gv

>>>>>>>> new file mode 100644

>>>>>>>> index 0000000..f8cb21a

>>>>>>>> --- /dev/null

>>>>>>>> +++ b/doc/images/timer_fsm.gv

>>>>>>>> @@ -0,0 +1,38 @@

>>>>>>>> +digraph timer_state_machine {

>>>>>>>> +       rankdir=LR;

>>>>>>>> +       size="12,20";

>>>>>>>> +       node [fontsize=28];

>>>>>>>> +       edge [fontsize=28];

>>>>>>>> +       node [shape=doublecircle]; Timer_Unalloc

>>>>>>>> +                                  Timeout_Unalloc

>>>>>>>> +                                  Timeout_Delivered;

>>>>>>>> +        node [shape=rectangle]; Timeout_Queued;

>>>>>>>> +       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 -> Timeout_Queued [label="=>odp_queue_enq()"];

>>>>>>>> +       Timeout_Queued -> 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];

>>>>>>>> +}

>>>>>>>> diff --git a/doc/users-guide/Makefile.am

>>>>>>>> b/doc/users-guide/Makefile.am

>>>>>>>> index 74caa96..6bb0131 100644

>>>>>>>> --- a/doc/users-guide/Makefile.am

>>>>>>>> +++ b/doc/users-guide/Makefile.am

>>>>>>>> @@ -30,6 +30,7 @@ IMAGES = $(top_srcdir)/doc/images/overview.svg \

>>>>>>>>          $(top_srcdir)/doc/images/release_git.svg \

>>>>>>>>          $(top_srcdir)/doc/images/segment.svg \

>>>>>>>>          $(top_srcdir)/doc/images/simple_release_git.svg \

>>>>>>>> +        $(top_srcdir)/doc/images/timer_fsm.svg \

>>>>>>>>          $(top_srcdir)/doc/images/tm_hierarchy.svg \

>>>>>>>>          $(top_srcdir)/doc/images/tm_node.svg

>>>>>>>>

>>>>>>>> --

>>>>>>>> 2.5.0

>>>>>>>>

>>>>>>>> _______________________________________________

>>>>>>>> lng-odp mailing list

>>>>>>>> lng-odp@lists.linaro.org

>>>>>>>> https://lists.linaro.org/mailman/listinfo/lng-odp

>>>>>>>>

>>>>>>>

>>>>>>>

>>>>>>

>>>>>

>>>>

>>>

>>

>
Christophe Milard May 12, 2016, 1:24 p.m. UTC | #9
Hi Bill,
Just sent you a v4 containing my proposal regarding the FSMs and the text
around it.
I still have a few comments:

First, your FSM mixed events and actions on the transition arrow. I could
not get graphviz to have 2 colors on the same transition (one for events,
one for actions), so I have restricted them to events. The only action
worth mentioning is the event queuing action which is clearly described by
your text (and a reminder in mine). The square node could be a way to
represent it, but I don't like mixing event and action undistinguished. As
long as it is clear and consistent (it seems I have a need for that these
days...), I won't bother you.

Then a few questions:

1)My Time-out FSM does not show what happens to a time-out event when an
odp_timer_free() is called while the timeout is in delivered state. I
assume the time-out remains in delivered state until a
odp_event_free/timeout_free() is called manually, but I am not sure (having
2 different FSM actally clarify this point). You are welcome to update my
FSMs if we should keep them.

2)Your text says "Following start, applications may allocate, set, cancel,
and free, timers...". I guess the comma after "timer" is a typo.

3)The last sentence in your text is: "However, upon receiving a timeout
event the application can use the odp_timeout_fresh() API to inquire
whether the timeout event is fresh or stale". What is the definition of a
stale timer?

4) And I kept the best for the end... :-)
What about the validity of the pointer returned by odp_timeout_user_ptr()
when the timer was set by odp thread A on a queue belonging to ODP thread B
(B != A). Interresting question isn't it? :-).

Thanks for reading...

Christophe.

On 12 May 2016 at 14:08, Bill Fischofer <bill.fischofer@linaro.org> wrote:

> The green is for Timer related transitions while the blue is for Timeout

> related transitions. This format also seemed to get rid of some of the

> annoying line crossings. The red arrows are the transitions that relate to

> timer expiration and resulting timeout event scheduling, neither of which

> involve Timer APIs. So the colors were supposed to make things easier to

> read.

>

> I'm all for improving clarity, so if you have a better way of diagraming

> this that's great. I believe the diagrams and text go together in either

> case and are intended to be taken together to improve understanding.  The

> goal of each of these sections of the User Guide should be to give readers

> confidence that they understand how to use these APIs in writing their own

> ODP applications, or for ODP implementers to understand the semantics they

> need to provide in their own implementations of them.

>

> On Thu, May 12, 2016 at 3:13 AM, Christophe Milard <

> christophe.milard@linaro.org> wrote:

>

>> Hi,

>>

>> I am still confused with this shared FSM for the 2 distinct objects: The

>> red seems to be actions associated with the transitions (and the timer

>> expiration event is no longer shown on the transition showing the actions).

>> I am not sure what the distinction between the green and blue events are.

>> I propose the following: I will send a complete proposal to you,

>> including both the FSMs and the text. If we still disagree on which one is

>> clearer, we'll let other reviewer decide (you/I would sent a v4 with my

>> stuff in). If no one does react, I am ok to mark your proposal as reviewed

>> because some doc is better than none.

>>

>> So you are sure to  win :-)

>>

>> Christophe.

>>

>> On 11 May 2016 at 01:47, Bill Fischofer <bill.fischofer@linaro.org>

>> wrote:

>>

>>> I looked at them and I liked the added color so reworked my diagram

>>> adding that and posted a v3 for it. I'm not sure trying to separate into

>>> two diagrams really helps but perhaps this reworked diagram is easier to

>>> follow?

>>>

>>> On Tue, May 10, 2016 at 3:45 AM, Christophe Milard <

>>> christophe.milard@linaro.org> wrote:

>>>

>>>> Hi Bill,

>>>>

>>>> Just sent you an RFC with 2 FSMs.

>>>> Had to fight a bit with graphviz. Not finished yet. I will continue

>>>> working on it if you think that makes sense...

>>>> Christophe.

>>>>

>>>> On 9 May 2016 at 17:15, Bill Fischofer <bill.fischofer@linaro.org>

>>>> wrote:

>>>>

>>>>> As we discussed in our call, we can both play around with graphviz and

>>>>> see if a prettier diagram can be constructed that is perhaps clearer,

>>>>> possibly with two separate FSMs.

>>>>>

>>>>> On Mon, May 9, 2016 at 9:39 AM, Christophe Milard <

>>>>> christophe.milard@linaro.org> wrote:

>>>>>

>>>>>>

>>>>>>

>>>>>> On 9 May 2016 at 16:28, Bill Fischofer <bill.fischofer@linaro.org>

>>>>>> wrote:

>>>>>>

>>>>>>>

>>>>>>>

>>>>>>> On Mon, May 9, 2016 at 7:58 AM, Christophe Milard <

>>>>>>> christophe.milard@linaro.org> wrote:

>>>>>>>

>>>>>>>> I am a bit confused by this diagram: It feels to me that timers and

>>>>>>>> timeout have separate lives (even , if of course some dependency exist).

>>>>>>>> From this diagram, it feels like these 2 separate objects (timers

>>>>>>>> and time out events) share states...? do they?

>>>>>>>>

>>>>>>>

>>>>>>> Actually they do, which is what this diagram is trying to express.

>>>>>>> When a timer is set one of the arguments is the timeout event that should

>>>>>>> be associated with it, so it is an error to attempt to free that event

>>>>>>> while it is associated with a set timer (results are undefined if you do).

>>>>>>> Timers are somewhat unique in this respect.

>>>>>>>

>>>>>>

>>>>>> Sorry, Bill I still don't get it: Aren't you saying here that these

>>>>>> are 2 separate FSMs, but that these 2 separate FSMs are actually

>>>>>> actionned/"triggered" by (partly) the same events? There is nothing wrong

>>>>>> with that... Then they should be represented as 2 separated FSM with the

>>>>>> same event names on the transitions triggered by the same events...

>>>>>>  Or I am very confused...

>>>>>>

>>>>>> Christophe.

>>>>>>

>>>>>>>

>>>>>>>

>>>>>>>> In my head, and from the understanding I had, it feels that the to

>>>>>>>> 4 top states are for timers, whereas time-out events have only 2 states:

>>>>>>>> Unallocated or allocated.

>>>>>>>> It feels you are doing 1 FSM out of two. Maybe , it should be two

>>>>>>>> FSM (keeping the same transition names to show the dependancy.)

>>>>>>>>

>>>>>>>> And I guess there is a typo at "odp_timeout_freee().

>>>>>>>>

>>>>>>>

>>>>>>> Thanks.  I'll fix that in the next version.

>>>>>>>

>>>>>>>

>>>>>>>>

>>>>>>>> Christophe

>>>>>>>>

>>>>>>>> On 7 May 2016 at 18:15, Bill Fischofer <bill.fischofer@linaro.org>

>>>>>>>> wrote:

>>>>>>>>

>>>>>>>>> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

>>>>>>>>> ---

>>>>>>>>>  doc/images/.gitignore       |  1 +

>>>>>>>>>  doc/images/timer_fsm.gv     | 38

>>>>>>>>> ++++++++++++++++++++++++++++++++++++++

>>>>>>>>>  doc/users-guide/Makefile.am |  1 +

>>>>>>>>>  3 files changed, 40 insertions(+)

>>>>>>>>>  create mode 100644 doc/images/timer_fsm.gv

>>>>>>>>>

>>>>>>>>> diff --git a/doc/images/.gitignore b/doc/images/.gitignore

>>>>>>>>> index a19aa75..72cf7ec 100644

>>>>>>>>> --- a/doc/images/.gitignore

>>>>>>>>> +++ b/doc/images/.gitignore

>>>>>>>>> @@ -1,2 +1,3 @@

>>>>>>>>>  resource_management.svg

>>>>>>>>>  pktio_fsm.svg

>>>>>>>>> +timer_fsm.svg

>>>>>>>>> diff --git a/doc/images/timer_fsm.gv b/doc/images/timer_fsm.gv

>>>>>>>>> new file mode 100644

>>>>>>>>> index 0000000..f8cb21a

>>>>>>>>> --- /dev/null

>>>>>>>>> +++ b/doc/images/timer_fsm.gv

>>>>>>>>> @@ -0,0 +1,38 @@

>>>>>>>>> +digraph timer_state_machine {

>>>>>>>>> +       rankdir=LR;

>>>>>>>>> +       size="12,20";

>>>>>>>>> +       node [fontsize=28];

>>>>>>>>> +       edge [fontsize=28];

>>>>>>>>> +       node [shape=doublecircle]; Timer_Unalloc

>>>>>>>>> +                                  Timeout_Unalloc

>>>>>>>>> +                                  Timeout_Delivered;

>>>>>>>>> +        node [shape=rectangle]; Timeout_Queued;

>>>>>>>>> +       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 -> Timeout_Queued [label="=>odp_queue_enq()"];

>>>>>>>>> +       Timeout_Queued -> 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];

>>>>>>>>> +}

>>>>>>>>> diff --git a/doc/users-guide/Makefile.am

>>>>>>>>> b/doc/users-guide/Makefile.am

>>>>>>>>> index 74caa96..6bb0131 100644

>>>>>>>>> --- a/doc/users-guide/Makefile.am

>>>>>>>>> +++ b/doc/users-guide/Makefile.am

>>>>>>>>> @@ -30,6 +30,7 @@ IMAGES = $(top_srcdir)/doc/images/overview.svg \

>>>>>>>>>          $(top_srcdir)/doc/images/release_git.svg \

>>>>>>>>>          $(top_srcdir)/doc/images/segment.svg \

>>>>>>>>>          $(top_srcdir)/doc/images/simple_release_git.svg \

>>>>>>>>> +        $(top_srcdir)/doc/images/timer_fsm.svg \

>>>>>>>>>          $(top_srcdir)/doc/images/tm_hierarchy.svg \

>>>>>>>>>          $(top_srcdir)/doc/images/tm_node.svg

>>>>>>>>>

>>>>>>>>> --

>>>>>>>>> 2.5.0

>>>>>>>>>

>>>>>>>>> _______________________________________________

>>>>>>>>> lng-odp mailing list

>>>>>>>>> lng-odp@lists.linaro.org

>>>>>>>>> https://lists.linaro.org/mailman/listinfo/lng-odp

>>>>>>>>>

>>>>>>>>

>>>>>>>>

>>>>>>>

>>>>>>

>>>>>

>>>>

>>>

>>

>
Bill Fischofer May 12, 2016, 5:23 p.m. UTC | #10
This looks good.  I've sent a v5 that slightly reformats the images to make
them consistent, but I'm happy with the two diagrams if that looks better
to you.

And I've now just read your other comments.  Let me look and see if a v6 is
needed to address them :)

On Thu, May 12, 2016 at 8:24 AM, Christophe Milard <
christophe.milard@linaro.org> wrote:

> Hi Bill,

> Just sent you a v4 containing my proposal regarding the FSMs and the text

> around it.

> I still have a few comments:

>

> First, your FSM mixed events and actions on the transition arrow. I could

> not get graphviz to have 2 colors on the same transition (one for events,

> one for actions), so I have restricted them to events. The only action

> worth mentioning is the event queuing action which is clearly described by

> your text (and a reminder in mine). The square node could be a way to

> represent it, but I don't like mixing event and action undistinguished. As

> long as it is clear and consistent (it seems I have a need for that these

> days...), I won't bother you.

>

> Then a few questions:

>

> 1)My Time-out FSM does not show what happens to a time-out event when an

> odp_timer_free() is called while the timeout is in delivered state. I

> assume the time-out remains in delivered state until a

> odp_event_free/timeout_free() is called manually, but I am not sure (having

> 2 different FSM actally clarify this point). You are welcome to update my

> FSMs if we should keep them.

>

> 2)Your text says "Following start, applications may allocate, set, cancel,

> and free, timers...". I guess the comma after "timer" is a typo.

>

> 3)The last sentence in your text is: "However, upon receiving a timeout

> event the application can use the odp_timeout_fresh() API to inquire

> whether the timeout event is fresh or stale". What is the definition of a

> stale timer?

>

> 4) And I kept the best for the end... :-)

> What about the validity of the pointer returned by odp_timeout_user_ptr()

> when the timer was set by odp thread A on a queue belonging to ODP thread B

> (B != A). Interresting question isn't it? :-).

>

> Thanks for reading...

>

> Christophe.

>

> On 12 May 2016 at 14:08, Bill Fischofer <bill.fischofer@linaro.org> wrote:

>

>> The green is for Timer related transitions while the blue is for Timeout

>> related transitions. This format also seemed to get rid of some of the

>> annoying line crossings. The red arrows are the transitions that relate to

>> timer expiration and resulting timeout event scheduling, neither of which

>> involve Timer APIs. So the colors were supposed to make things easier to

>> read.

>>

>> I'm all for improving clarity, so if you have a better way of diagraming

>> this that's great. I believe the diagrams and text go together in either

>> case and are intended to be taken together to improve understanding.  The

>> goal of each of these sections of the User Guide should be to give readers

>> confidence that they understand how to use these APIs in writing their own

>> ODP applications, or for ODP implementers to understand the semantics they

>> need to provide in their own implementations of them.

>>

>> On Thu, May 12, 2016 at 3:13 AM, Christophe Milard <

>> christophe.milard@linaro.org> wrote:

>>

>>> Hi,

>>>

>>> I am still confused with this shared FSM for the 2 distinct objects: The

>>> red seems to be actions associated with the transitions (and the timer

>>> expiration event is no longer shown on the transition showing the actions).

>>> I am not sure what the distinction between the green and blue events are.

>>> I propose the following: I will send a complete proposal to you,

>>> including both the FSMs and the text. If we still disagree on which one is

>>> clearer, we'll let other reviewer decide (you/I would sent a v4 with my

>>> stuff in). If no one does react, I am ok to mark your proposal as reviewed

>>> because some doc is better than none.

>>>

>>> So you are sure to  win :-)

>>>

>>> Christophe.

>>>

>>> On 11 May 2016 at 01:47, Bill Fischofer <bill.fischofer@linaro.org>

>>> wrote:

>>>

>>>> I looked at them and I liked the added color so reworked my diagram

>>>> adding that and posted a v3 for it. I'm not sure trying to separate into

>>>> two diagrams really helps but perhaps this reworked diagram is easier to

>>>> follow?

>>>>

>>>> On Tue, May 10, 2016 at 3:45 AM, Christophe Milard <

>>>> christophe.milard@linaro.org> wrote:

>>>>

>>>>> Hi Bill,

>>>>>

>>>>> Just sent you an RFC with 2 FSMs.

>>>>> Had to fight a bit with graphviz. Not finished yet. I will continue

>>>>> working on it if you think that makes sense...

>>>>> Christophe.

>>>>>

>>>>> On 9 May 2016 at 17:15, Bill Fischofer <bill.fischofer@linaro.org>

>>>>> wrote:

>>>>>

>>>>>> As we discussed in our call, we can both play around with graphviz

>>>>>> and see if a prettier diagram can be constructed that is perhaps clearer,

>>>>>> possibly with two separate FSMs.

>>>>>>

>>>>>> On Mon, May 9, 2016 at 9:39 AM, Christophe Milard <

>>>>>> christophe.milard@linaro.org> wrote:

>>>>>>

>>>>>>>

>>>>>>>

>>>>>>> On 9 May 2016 at 16:28, Bill Fischofer <bill.fischofer@linaro.org>

>>>>>>> wrote:

>>>>>>>

>>>>>>>>

>>>>>>>>

>>>>>>>> On Mon, May 9, 2016 at 7:58 AM, Christophe Milard <

>>>>>>>> christophe.milard@linaro.org> wrote:

>>>>>>>>

>>>>>>>>> I am a bit confused by this diagram: It feels to me that timers

>>>>>>>>> and timeout have separate lives (even , if of course some dependency

>>>>>>>>> exist).

>>>>>>>>> From this diagram, it feels like these 2 separate objects (timers

>>>>>>>>> and time out events) share states...? do they?

>>>>>>>>>

>>>>>>>>

>>>>>>>> Actually they do, which is what this diagram is trying to express.

>>>>>>>> When a timer is set one of the arguments is the timeout event that should

>>>>>>>> be associated with it, so it is an error to attempt to free that event

>>>>>>>> while it is associated with a set timer (results are undefined if you do).

>>>>>>>> Timers are somewhat unique in this respect.

>>>>>>>>

>>>>>>>

>>>>>>> Sorry, Bill I still don't get it: Aren't you saying here that these

>>>>>>> are 2 separate FSMs, but that these 2 separate FSMs are actually

>>>>>>> actionned/"triggered" by (partly) the same events? There is nothing wrong

>>>>>>> with that... Then they should be represented as 2 separated FSM with the

>>>>>>> same event names on the transitions triggered by the same events...

>>>>>>>  Or I am very confused...

>>>>>>>

>>>>>>> Christophe.

>>>>>>>

>>>>>>>>

>>>>>>>>

>>>>>>>>> In my head, and from the understanding I had, it feels that the to

>>>>>>>>> 4 top states are for timers, whereas time-out events have only 2 states:

>>>>>>>>> Unallocated or allocated.

>>>>>>>>> It feels you are doing 1 FSM out of two. Maybe , it should be two

>>>>>>>>> FSM (keeping the same transition names to show the dependancy.)

>>>>>>>>>

>>>>>>>>> And I guess there is a typo at "odp_timeout_freee().

>>>>>>>>>

>>>>>>>>

>>>>>>>> Thanks.  I'll fix that in the next version.

>>>>>>>>

>>>>>>>>

>>>>>>>>>

>>>>>>>>> Christophe

>>>>>>>>>

>>>>>>>>> On 7 May 2016 at 18:15, Bill Fischofer <bill.fischofer@linaro.org>

>>>>>>>>> wrote:

>>>>>>>>>

>>>>>>>>>> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

>>>>>>>>>> ---

>>>>>>>>>>  doc/images/.gitignore       |  1 +

>>>>>>>>>>  doc/images/timer_fsm.gv     | 38

>>>>>>>>>> ++++++++++++++++++++++++++++++++++++++

>>>>>>>>>>  doc/users-guide/Makefile.am |  1 +

>>>>>>>>>>  3 files changed, 40 insertions(+)

>>>>>>>>>>  create mode 100644 doc/images/timer_fsm.gv

>>>>>>>>>>

>>>>>>>>>> diff --git a/doc/images/.gitignore b/doc/images/.gitignore

>>>>>>>>>> index a19aa75..72cf7ec 100644

>>>>>>>>>> --- a/doc/images/.gitignore

>>>>>>>>>> +++ b/doc/images/.gitignore

>>>>>>>>>> @@ -1,2 +1,3 @@

>>>>>>>>>>  resource_management.svg

>>>>>>>>>>  pktio_fsm.svg

>>>>>>>>>> +timer_fsm.svg

>>>>>>>>>> diff --git a/doc/images/timer_fsm.gv b/doc/images/timer_fsm.gv

>>>>>>>>>> new file mode 100644

>>>>>>>>>> index 0000000..f8cb21a

>>>>>>>>>> --- /dev/null

>>>>>>>>>> +++ b/doc/images/timer_fsm.gv

>>>>>>>>>> @@ -0,0 +1,38 @@

>>>>>>>>>> +digraph timer_state_machine {

>>>>>>>>>> +       rankdir=LR;

>>>>>>>>>> +       size="12,20";

>>>>>>>>>> +       node [fontsize=28];

>>>>>>>>>> +       edge [fontsize=28];

>>>>>>>>>> +       node [shape=doublecircle]; Timer_Unalloc

>>>>>>>>>> +                                  Timeout_Unalloc

>>>>>>>>>> +                                  Timeout_Delivered;

>>>>>>>>>> +        node [shape=rectangle]; Timeout_Queued;

>>>>>>>>>> +       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 -> Timeout_Queued [label="=>odp_queue_enq()"];

>>>>>>>>>> +       Timeout_Queued -> 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];

>>>>>>>>>> +}

>>>>>>>>>> diff --git a/doc/users-guide/Makefile.am

>>>>>>>>>> b/doc/users-guide/Makefile.am

>>>>>>>>>> index 74caa96..6bb0131 100644

>>>>>>>>>> --- a/doc/users-guide/Makefile.am

>>>>>>>>>> +++ b/doc/users-guide/Makefile.am

>>>>>>>>>> @@ -30,6 +30,7 @@ IMAGES = $(top_srcdir)/doc/images/overview.svg \

>>>>>>>>>>          $(top_srcdir)/doc/images/release_git.svg \

>>>>>>>>>>          $(top_srcdir)/doc/images/segment.svg \

>>>>>>>>>>          $(top_srcdir)/doc/images/simple_release_git.svg \

>>>>>>>>>> +        $(top_srcdir)/doc/images/timer_fsm.svg \

>>>>>>>>>>          $(top_srcdir)/doc/images/tm_hierarchy.svg \

>>>>>>>>>>          $(top_srcdir)/doc/images/tm_node.svg

>>>>>>>>>>

>>>>>>>>>> --

>>>>>>>>>> 2.5.0

>>>>>>>>>>

>>>>>>>>>> _______________________________________________

>>>>>>>>>> lng-odp mailing list

>>>>>>>>>> lng-odp@lists.linaro.org

>>>>>>>>>> https://lists.linaro.org/mailman/listinfo/lng-odp

>>>>>>>>>>

>>>>>>>>>

>>>>>>>>>

>>>>>>>>

>>>>>>>

>>>>>>

>>>>>

>>>>

>>>

>>

>
Bill Fischofer May 12, 2016, 5:44 p.m. UTC | #11
On Thu, May 12, 2016 at 8:24 AM, Christophe Milard <
christophe.milard@linaro.org> wrote:

> Hi Bill,

> Just sent you a v4 containing my proposal regarding the FSMs and the text

> around it.

> I still have a few comments:

>

> First, your FSM mixed events and actions on the transition arrow. I could

> not get graphviz to have 2 colors on the same transition (one for events,

> one for actions), so I have restricted them to events. The only action

> worth mentioning is the event queuing action which is clearly described by

> your text (and a reminder in mine). The square node could be a way to

> represent it, but I don't like mixing event and action undistinguished. As

> long as it is clear and consistent (it seems I have a need for that these

> days...), I won't bother you.

>


I think it's fine this way.  The expiration/schedule call adds detail
because a timeout isn't actually delivered until the scheduler returns it
to a thread, but I think this is clear enough here.


>

> Then a few questions:

>

> 1)My Time-out FSM does not show what happens to a time-out event when an

> odp_timer_free() is called while the timeout is in delivered state. I

> assume the time-out remains in delivered state until a

> odp_event_free/timeout_free() is called manually, but I am not sure (having

> 2 different FSM actally clarify this point). You are welcome to update my

> FSMs if we should keep them.

>


Nothing happens to the event since the association between timer and
timeout ends as soon as the timer expires. At that point the timer is in
the "Timer_Expired" state and the Timeout moves to the "Timeout_Delivered"
state (Timeout_Pending in my previous diagram). So they're no longer
coupled and operations on one no longer affect the other.


>

> 2)Your text says "Following start, applications may allocate, set, cancel,

> and free, timers...". I guess the comma after "timer" is a typo.

>


Yes, thanks for the correction.


>

> 3)The last sentence in your text is: "However, upon receiving a timeout

> event the application can use the odp_timeout_fresh() API to inquire

> whether the timeout event is fresh or stale". What is the definition of a

> stale timer?

>


This was a back reference to the earlier sentence discussing
odp_timer_cancel(): "An attempted cancel will fail if the timer is not set
or if it has already
expired."  However, I recall this was an area that Ola and Petri debated
extensively when these were first defined. There is an implicit imprecision
in any efficient timer system since independent threads can be setting,
resetting, and cancelling timers while they are "in flight". Generally when
a timeout event is received it's the responsibility of the application to
determine whether that event is meaningful or not based on other
information it has (e.g., it's a TCP delayed ACK timer, but we've just
received a packet on that connection so I should ignore this event, etc.).
 odp_timer_fresh() is a hook that allows the implementation to communicate
staleness information if it knows something that the application might not
otherwise know, but I expect most applications will just ignore this as a
reported "fresh" timer may still not be meaningful from the application's
perspective for reasons it knows.


>

> 4) And I kept the best for the end... :-)

> What about the validity of the pointer returned by odp_timeout_user_ptr()

> when the timer was set by odp thread A on a queue belonging to ODP thread B

> (B != A). Interresting question isn't it? :-).

>

>

The user ptr should really be revised for Tiger Moth to include a length
(just as we did for queues) so that it can be part of scheduler
prefetching. But as you know Monarch assumes everything is running in the
same address space.  Make a note of this (and similar context pointers) as
this is something we need to cover in Tiger Moth process discussions.



> Thanks for reading...

>



After now reading this I don't think a v6 is needed unless you do.


>

> Christophe.

>

> On 12 May 2016 at 14:08, Bill Fischofer <bill.fischofer@linaro.org> wrote:

>

>> The green is for Timer related transitions while the blue is for Timeout

>> related transitions. This format also seemed to get rid of some of the

>> annoying line crossings. The red arrows are the transitions that relate to

>> timer expiration and resulting timeout event scheduling, neither of which

>> involve Timer APIs. So the colors were supposed to make things easier to

>> read.

>>

>> I'm all for improving clarity, so if you have a better way of diagraming

>> this that's great. I believe the diagrams and text go together in either

>> case and are intended to be taken together to improve understanding.  The

>> goal of each of these sections of the User Guide should be to give readers

>> confidence that they understand how to use these APIs in writing their own

>> ODP applications, or for ODP implementers to understand the semantics they

>> need to provide in their own implementations of them.

>>

>> On Thu, May 12, 2016 at 3:13 AM, Christophe Milard <

>> christophe.milard@linaro.org> wrote:

>>

>>> Hi,

>>>

>>> I am still confused with this shared FSM for the 2 distinct objects: The

>>> red seems to be actions associated with the transitions (and the timer

>>> expiration event is no longer shown on the transition showing the actions).

>>> I am not sure what the distinction between the green and blue events are.

>>> I propose the following: I will send a complete proposal to you,

>>> including both the FSMs and the text. If we still disagree on which one is

>>> clearer, we'll let other reviewer decide (you/I would sent a v4 with my

>>> stuff in). If no one does react, I am ok to mark your proposal as reviewed

>>> because some doc is better than none.

>>>

>>> So you are sure to  win :-)

>>>

>>> Christophe.

>>>

>>> On 11 May 2016 at 01:47, Bill Fischofer <bill.fischofer@linaro.org>

>>> wrote:

>>>

>>>> I looked at them and I liked the added color so reworked my diagram

>>>> adding that and posted a v3 for it. I'm not sure trying to separate into

>>>> two diagrams really helps but perhaps this reworked diagram is easier to

>>>> follow?

>>>>

>>>> On Tue, May 10, 2016 at 3:45 AM, Christophe Milard <

>>>> christophe.milard@linaro.org> wrote:

>>>>

>>>>> Hi Bill,

>>>>>

>>>>> Just sent you an RFC with 2 FSMs.

>>>>> Had to fight a bit with graphviz. Not finished yet. I will continue

>>>>> working on it if you think that makes sense...

>>>>> Christophe.

>>>>>

>>>>> On 9 May 2016 at 17:15, Bill Fischofer <bill.fischofer@linaro.org>

>>>>> wrote:

>>>>>

>>>>>> As we discussed in our call, we can both play around with graphviz

>>>>>> and see if a prettier diagram can be constructed that is perhaps clearer,

>>>>>> possibly with two separate FSMs.

>>>>>>

>>>>>> On Mon, May 9, 2016 at 9:39 AM, Christophe Milard <

>>>>>> christophe.milard@linaro.org> wrote:

>>>>>>

>>>>>>>

>>>>>>>

>>>>>>> On 9 May 2016 at 16:28, Bill Fischofer <bill.fischofer@linaro.org>

>>>>>>> wrote:

>>>>>>>

>>>>>>>>

>>>>>>>>

>>>>>>>> On Mon, May 9, 2016 at 7:58 AM, Christophe Milard <

>>>>>>>> christophe.milard@linaro.org> wrote:

>>>>>>>>

>>>>>>>>> I am a bit confused by this diagram: It feels to me that timers

>>>>>>>>> and timeout have separate lives (even , if of course some dependency

>>>>>>>>> exist).

>>>>>>>>> From this diagram, it feels like these 2 separate objects (timers

>>>>>>>>> and time out events) share states...? do they?

>>>>>>>>>

>>>>>>>>

>>>>>>>> Actually they do, which is what this diagram is trying to express.

>>>>>>>> When a timer is set one of the arguments is the timeout event that should

>>>>>>>> be associated with it, so it is an error to attempt to free that event

>>>>>>>> while it is associated with a set timer (results are undefined if you do).

>>>>>>>> Timers are somewhat unique in this respect.

>>>>>>>>

>>>>>>>

>>>>>>> Sorry, Bill I still don't get it: Aren't you saying here that these

>>>>>>> are 2 separate FSMs, but that these 2 separate FSMs are actually

>>>>>>> actionned/"triggered" by (partly) the same events? There is nothing wrong

>>>>>>> with that... Then they should be represented as 2 separated FSM with the

>>>>>>> same event names on the transitions triggered by the same events...

>>>>>>>  Or I am very confused...

>>>>>>>

>>>>>>> Christophe.

>>>>>>>

>>>>>>>>

>>>>>>>>

>>>>>>>>> In my head, and from the understanding I had, it feels that the to

>>>>>>>>> 4 top states are for timers, whereas time-out events have only 2 states:

>>>>>>>>> Unallocated or allocated.

>>>>>>>>> It feels you are doing 1 FSM out of two. Maybe , it should be two

>>>>>>>>> FSM (keeping the same transition names to show the dependancy.)

>>>>>>>>>

>>>>>>>>> And I guess there is a typo at "odp_timeout_freee().

>>>>>>>>>

>>>>>>>>

>>>>>>>> Thanks.  I'll fix that in the next version.

>>>>>>>>

>>>>>>>>

>>>>>>>>>

>>>>>>>>> Christophe

>>>>>>>>>

>>>>>>>>> On 7 May 2016 at 18:15, Bill Fischofer <bill.fischofer@linaro.org>

>>>>>>>>> wrote:

>>>>>>>>>

>>>>>>>>>> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

>>>>>>>>>> ---

>>>>>>>>>>  doc/images/.gitignore       |  1 +

>>>>>>>>>>  doc/images/timer_fsm.gv     | 38

>>>>>>>>>> ++++++++++++++++++++++++++++++++++++++

>>>>>>>>>>  doc/users-guide/Makefile.am |  1 +

>>>>>>>>>>  3 files changed, 40 insertions(+)

>>>>>>>>>>  create mode 100644 doc/images/timer_fsm.gv

>>>>>>>>>>

>>>>>>>>>> diff --git a/doc/images/.gitignore b/doc/images/.gitignore

>>>>>>>>>> index a19aa75..72cf7ec 100644

>>>>>>>>>> --- a/doc/images/.gitignore

>>>>>>>>>> +++ b/doc/images/.gitignore

>>>>>>>>>> @@ -1,2 +1,3 @@

>>>>>>>>>>  resource_management.svg

>>>>>>>>>>  pktio_fsm.svg

>>>>>>>>>> +timer_fsm.svg

>>>>>>>>>> diff --git a/doc/images/timer_fsm.gv b/doc/images/timer_fsm.gv

>>>>>>>>>> new file mode 100644

>>>>>>>>>> index 0000000..f8cb21a

>>>>>>>>>> --- /dev/null

>>>>>>>>>> +++ b/doc/images/timer_fsm.gv

>>>>>>>>>> @@ -0,0 +1,38 @@

>>>>>>>>>> +digraph timer_state_machine {

>>>>>>>>>> +       rankdir=LR;

>>>>>>>>>> +       size="12,20";

>>>>>>>>>> +       node [fontsize=28];

>>>>>>>>>> +       edge [fontsize=28];

>>>>>>>>>> +       node [shape=doublecircle]; Timer_Unalloc

>>>>>>>>>> +                                  Timeout_Unalloc

>>>>>>>>>> +                                  Timeout_Delivered;

>>>>>>>>>> +        node [shape=rectangle]; Timeout_Queued;

>>>>>>>>>> +       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 -> Timeout_Queued [label="=>odp_queue_enq()"];

>>>>>>>>>> +       Timeout_Queued -> 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];

>>>>>>>>>> +}

>>>>>>>>>> diff --git a/doc/users-guide/Makefile.am

>>>>>>>>>> b/doc/users-guide/Makefile.am

>>>>>>>>>> index 74caa96..6bb0131 100644

>>>>>>>>>> --- a/doc/users-guide/Makefile.am

>>>>>>>>>> +++ b/doc/users-guide/Makefile.am

>>>>>>>>>> @@ -30,6 +30,7 @@ IMAGES = $(top_srcdir)/doc/images/overview.svg \

>>>>>>>>>>          $(top_srcdir)/doc/images/release_git.svg \

>>>>>>>>>>          $(top_srcdir)/doc/images/segment.svg \

>>>>>>>>>>          $(top_srcdir)/doc/images/simple_release_git.svg \

>>>>>>>>>> +        $(top_srcdir)/doc/images/timer_fsm.svg \

>>>>>>>>>>          $(top_srcdir)/doc/images/tm_hierarchy.svg \

>>>>>>>>>>          $(top_srcdir)/doc/images/tm_node.svg

>>>>>>>>>>

>>>>>>>>>> --

>>>>>>>>>> 2.5.0

>>>>>>>>>>

>>>>>>>>>> _______________________________________________

>>>>>>>>>> lng-odp mailing list

>>>>>>>>>> lng-odp@lists.linaro.org

>>>>>>>>>> https://lists.linaro.org/mailman/listinfo/lng-odp

>>>>>>>>>>

>>>>>>>>>

>>>>>>>>>

>>>>>>>>

>>>>>>>

>>>>>>

>>>>>

>>>>

>>>

>>

>
Christophe Milard May 13, 2016, 9:50 a.m. UTC | #12
On 12 May 2016 at 19:44, Bill Fischofer <bill.fischofer@linaro.org> wrote:

>

>

> On Thu, May 12, 2016 at 8:24 AM, Christophe Milard <

> christophe.milard@linaro.org> wrote:

>

>> Hi Bill,

>> Just sent you a v4 containing my proposal regarding the FSMs and the text

>> around it.

>> I still have a few comments:

>>

>> First, your FSM mixed events and actions on the transition arrow. I could

>> not get graphviz to have 2 colors on the same transition (one for events,

>> one for actions), so I have restricted them to events. The only action

>> worth mentioning is the event queuing action which is clearly described by

>> your text (and a reminder in mine). The square node could be a way to

>> represent it, but I don't like mixing event and action undistinguished. As

>> long as it is clear and consistent (it seems I have a need for that these

>> days...), I won't bother you.

>>

>

> I think it's fine this way.  The expiration/schedule call adds detail

> because a timeout isn't actually delivered until the scheduler returns it

> to a thread, but I think this is clear enough here.

>

>


On V5:
Maybe the error originated from me (?), but I see that the TO_Delivered
node is double ringed: Double ringed node would show the start state, so I
think it shouldn't be double ringed. If the intention was to show the final
state (though there isn't really one here: a TO event can be reused I
assume), then the Timer_Expired state should be double ringed as well. But
as mentionned, as I cannot clearly see any final state here, I would simply
remove the double ring from the TO_delivered state.

>

>> Then a few questions:

>>

>> 1)My Time-out FSM does not show what happens to a time-out event when an

>> odp_timer_free() is called while the timeout is in delivered state. I

>> assume the time-out remains in delivered state until a

>> odp_event_free/timeout_free() is called manually, but I am not sure (having

>> 2 different FSM actally clarify this point). You are welcome to update my

>> FSMs if we should keep them.

>>

>

> Nothing happens to the event since the association between timer and

> timeout ends as soon as the timer expires. At that point the timer is in

> the "Timer_Expired" state and the Timeout moves to the "Timeout_Delivered"

> state (Timeout_Pending in my previous diagram). So they're no longer

> coupled and operations on one no longer affect the other.

>


In that case there should be a transition TO_delivered->TO_delivered
labeled odp_timer_free()


>

>

>>

>> 2)Your text says "Following start, applications may allocate, set,

>> cancel, and free, timers...". I guess the comma after "timer" is a typo.

>>

>

> Yes, thanks for the correction.

>

>

>>

>> 3)The last sentence in your text is: "However, upon receiving a timeout

>> event the application can use the odp_timeout_fresh() API to inquire

>> whether the timeout event is fresh or stale". What is the definition of a

>> stale timer?

>>

>

> This was a back reference to the earlier sentence discussing

> odp_timer_cancel(): "An attempted cancel will fail if the timer is not set

> or if it has already

> expired."  However, I recall this was an area that Ola and Petri debated

> extensively when these were first defined. There is an implicit imprecision

> in any efficient timer system since independent threads can be setting,

> resetting, and cancelling timers while they are "in flight". Generally when

> a timeout event is received it's the responsibility of the application to

> determine whether that event is meaningful or not based on other

> information it has (e.g., it's a TCP delayed ACK timer, but we've just

> received a packet on that connection so I should ignore this event, etc.).

>  odp_timer_fresh() is a hook that allows the implementation to communicate

> staleness information if it knows something that the application might not

> otherwise know, but I expect most applications will just ignore this as a

> reported "fresh" timer may still not be meaningful from the application's

> perspective for reasons it knows.

>


Not sure I understand that. I'll need explanations, and I guess the doc as
well.

>

>

>>

>> 4) And I kept the best for the end... :-)

>> What about the validity of the pointer returned by odp_timeout_user_ptr()

>> when the timer was set by odp thread A on a queue belonging to ODP thread B

>> (B != A). Interresting question isn't it? :-).

>>

>>

> The user ptr should really be revised for Tiger Moth to include a length

> (just as we did for queues) so that it can be part of scheduler

> prefetching. But as you know Monarch assumes everything is running in the

> same address space.  Make a note of this (and similar context pointers) as

> this is something we need to cover in Tiger Moth process discussions.

>


OK. Pity you are right here as I saw this as a good opportunity to trigger
some more opinions regarding the the process/thread debate... But you are
right: the aim is Monarch, and monarch just goes thread, so I cannot stop
you there :-)

I suggest we take a small HO as soon as possible (ping me when you want) so
you can explain the thing I don't understand and we can agree on a phrasing
around it on the fly.
Sorry, I see a need for V6, but I'll be on your side to make it the last
one :-)

Christophe.

>

>

>

>> Thanks for reading...

>>

>

>

> After now reading this I don't think a v6 is needed unless you do.

>

>

>>

>> Christophe.

>>

>> On 12 May 2016 at 14:08, Bill Fischofer <bill.fischofer@linaro.org>

>> wrote:

>>

>>> The green is for Timer related transitions while the blue is for Timeout

>>> related transitions. This format also seemed to get rid of some of the

>>> annoying line crossings. The red arrows are the transitions that relate to

>>> timer expiration and resulting timeout event scheduling, neither of which

>>> involve Timer APIs. So the colors were supposed to make things easier to

>>> read.

>>>

>>> I'm all for improving clarity, so if you have a better way of diagraming

>>> this that's great. I believe the diagrams and text go together in either

>>> case and are intended to be taken together to improve understanding.  The

>>> goal of each of these sections of the User Guide should be to give readers

>>> confidence that they understand how to use these APIs in writing their own

>>> ODP applications, or for ODP implementers to understand the semantics they

>>> need to provide in their own implementations of them.

>>>

>>> On Thu, May 12, 2016 at 3:13 AM, Christophe Milard <

>>> christophe.milard@linaro.org> wrote:

>>>

>>>> Hi,

>>>>

>>>> I am still confused with this shared FSM for the 2 distinct objects:

>>>> The red seems to be actions associated with the transitions (and the timer

>>>> expiration event is no longer shown on the transition showing the actions).

>>>> I am not sure what the distinction between the green and blue events are.

>>>> I propose the following: I will send a complete proposal to you,

>>>> including both the FSMs and the text. If we still disagree on which one is

>>>> clearer, we'll let other reviewer decide (you/I would sent a v4 with my

>>>> stuff in). If no one does react, I am ok to mark your proposal as reviewed

>>>> because some doc is better than none.

>>>>

>>>> So you are sure to  win :-)

>>>>

>>>> Christophe.

>>>>

>>>> On 11 May 2016 at 01:47, Bill Fischofer <bill.fischofer@linaro.org>

>>>> wrote:

>>>>

>>>>> I looked at them and I liked the added color so reworked my diagram

>>>>> adding that and posted a v3 for it. I'm not sure trying to separate into

>>>>> two diagrams really helps but perhaps this reworked diagram is easier to

>>>>> follow?

>>>>>

>>>>> On Tue, May 10, 2016 at 3:45 AM, Christophe Milard <

>>>>> christophe.milard@linaro.org> wrote:

>>>>>

>>>>>> Hi Bill,

>>>>>>

>>>>>> Just sent you an RFC with 2 FSMs.

>>>>>> Had to fight a bit with graphviz. Not finished yet. I will continue

>>>>>> working on it if you think that makes sense...

>>>>>> Christophe.

>>>>>>

>>>>>> On 9 May 2016 at 17:15, Bill Fischofer <bill.fischofer@linaro.org>

>>>>>> wrote:

>>>>>>

>>>>>>> As we discussed in our call, we can both play around with graphviz

>>>>>>> and see if a prettier diagram can be constructed that is perhaps clearer,

>>>>>>> possibly with two separate FSMs.

>>>>>>>

>>>>>>> On Mon, May 9, 2016 at 9:39 AM, Christophe Milard <

>>>>>>> christophe.milard@linaro.org> wrote:

>>>>>>>

>>>>>>>>

>>>>>>>>

>>>>>>>> On 9 May 2016 at 16:28, Bill Fischofer <bill.fischofer@linaro.org>

>>>>>>>> wrote:

>>>>>>>>

>>>>>>>>>

>>>>>>>>>

>>>>>>>>> On Mon, May 9, 2016 at 7:58 AM, Christophe Milard <

>>>>>>>>> christophe.milard@linaro.org> wrote:

>>>>>>>>>

>>>>>>>>>> I am a bit confused by this diagram: It feels to me that timers

>>>>>>>>>> and timeout have separate lives (even , if of course some dependency

>>>>>>>>>> exist).

>>>>>>>>>> From this diagram, it feels like these 2 separate objects (timers

>>>>>>>>>> and time out events) share states...? do they?

>>>>>>>>>>

>>>>>>>>>

>>>>>>>>> Actually they do, which is what this diagram is trying to express.

>>>>>>>>> When a timer is set one of the arguments is the timeout event that should

>>>>>>>>> be associated with it, so it is an error to attempt to free that event

>>>>>>>>> while it is associated with a set timer (results are undefined if you do).

>>>>>>>>> Timers are somewhat unique in this respect.

>>>>>>>>>

>>>>>>>>

>>>>>>>> Sorry, Bill I still don't get it: Aren't you saying here that these

>>>>>>>> are 2 separate FSMs, but that these 2 separate FSMs are actually

>>>>>>>> actionned/"triggered" by (partly) the same events? There is nothing wrong

>>>>>>>> with that... Then they should be represented as 2 separated FSM with the

>>>>>>>> same event names on the transitions triggered by the same events...

>>>>>>>>  Or I am very confused...

>>>>>>>>

>>>>>>>> Christophe.

>>>>>>>>

>>>>>>>>>

>>>>>>>>>

>>>>>>>>>> In my head, and from the understanding I had, it feels that the

>>>>>>>>>> to 4 top states are for timers, whereas time-out events have only 2 states:

>>>>>>>>>> Unallocated or allocated.

>>>>>>>>>> It feels you are doing 1 FSM out of two. Maybe , it should be two

>>>>>>>>>> FSM (keeping the same transition names to show the dependancy.)

>>>>>>>>>>

>>>>>>>>>> And I guess there is a typo at "odp_timeout_freee().

>>>>>>>>>>

>>>>>>>>>

>>>>>>>>> Thanks.  I'll fix that in the next version.

>>>>>>>>>

>>>>>>>>>

>>>>>>>>>>

>>>>>>>>>> Christophe

>>>>>>>>>>

>>>>>>>>>> On 7 May 2016 at 18:15, Bill Fischofer <bill.fischofer@linaro.org

>>>>>>>>>> > wrote:

>>>>>>>>>>

>>>>>>>>>>> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

>>>>>>>>>>> ---

>>>>>>>>>>>  doc/images/.gitignore       |  1 +

>>>>>>>>>>>  doc/images/timer_fsm.gv     | 38

>>>>>>>>>>> ++++++++++++++++++++++++++++++++++++++

>>>>>>>>>>>  doc/users-guide/Makefile.am |  1 +

>>>>>>>>>>>  3 files changed, 40 insertions(+)

>>>>>>>>>>>  create mode 100644 doc/images/timer_fsm.gv

>>>>>>>>>>>

>>>>>>>>>>> diff --git a/doc/images/.gitignore b/doc/images/.gitignore

>>>>>>>>>>> index a19aa75..72cf7ec 100644

>>>>>>>>>>> --- a/doc/images/.gitignore

>>>>>>>>>>> +++ b/doc/images/.gitignore

>>>>>>>>>>> @@ -1,2 +1,3 @@

>>>>>>>>>>>  resource_management.svg

>>>>>>>>>>>  pktio_fsm.svg

>>>>>>>>>>> +timer_fsm.svg

>>>>>>>>>>> diff --git a/doc/images/timer_fsm.gv b/doc/images/timer_fsm.gv

>>>>>>>>>>> new file mode 100644

>>>>>>>>>>> index 0000000..f8cb21a

>>>>>>>>>>> --- /dev/null

>>>>>>>>>>> +++ b/doc/images/timer_fsm.gv

>>>>>>>>>>> @@ -0,0 +1,38 @@

>>>>>>>>>>> +digraph timer_state_machine {

>>>>>>>>>>> +       rankdir=LR;

>>>>>>>>>>> +       size="12,20";

>>>>>>>>>>> +       node [fontsize=28];

>>>>>>>>>>> +       edge [fontsize=28];

>>>>>>>>>>> +       node [shape=doublecircle]; Timer_Unalloc

>>>>>>>>>>> +                                  Timeout_Unalloc

>>>>>>>>>>> +                                  Timeout_Delivered;

>>>>>>>>>>> +        node [shape=rectangle]; Timeout_Queued;

>>>>>>>>>>> +       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 -> Timeout_Queued [label="=>odp_queue_enq()"];

>>>>>>>>>>> +       Timeout_Queued -> 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];

>>>>>>>>>>> +}

>>>>>>>>>>> diff --git a/doc/users-guide/Makefile.am

>>>>>>>>>>> b/doc/users-guide/Makefile.am

>>>>>>>>>>> index 74caa96..6bb0131 100644

>>>>>>>>>>> --- a/doc/users-guide/Makefile.am

>>>>>>>>>>> +++ b/doc/users-guide/Makefile.am

>>>>>>>>>>> @@ -30,6 +30,7 @@ IMAGES = $(top_srcdir)/doc/images/overview.svg

>>>>>>>>>>> \

>>>>>>>>>>>          $(top_srcdir)/doc/images/release_git.svg \

>>>>>>>>>>>          $(top_srcdir)/doc/images/segment.svg \

>>>>>>>>>>>          $(top_srcdir)/doc/images/simple_release_git.svg \

>>>>>>>>>>> +        $(top_srcdir)/doc/images/timer_fsm.svg \

>>>>>>>>>>>          $(top_srcdir)/doc/images/tm_hierarchy.svg \

>>>>>>>>>>>          $(top_srcdir)/doc/images/tm_node.svg

>>>>>>>>>>>

>>>>>>>>>>> --

>>>>>>>>>>> 2.5.0

>>>>>>>>>>>

>>>>>>>>>>> _______________________________________________

>>>>>>>>>>> lng-odp mailing list

>>>>>>>>>>> lng-odp@lists.linaro.org

>>>>>>>>>>> https://lists.linaro.org/mailman/listinfo/lng-odp

>>>>>>>>>>>

>>>>>>>>>>

>>>>>>>>>>

>>>>>>>>>

>>>>>>>>

>>>>>>>

>>>>>>

>>>>>

>>>>

>>>

>>

>
Bill Fischofer May 13, 2016, 11:46 a.m. UTC | #13
On Fri, May 13, 2016 at 4:50 AM, Christophe Milard <
christophe.milard@linaro.org> wrote:

>

>

> On 12 May 2016 at 19:44, Bill Fischofer <bill.fischofer@linaro.org> wrote:

>

>>

>>

>> On Thu, May 12, 2016 at 8:24 AM, Christophe Milard <

>> christophe.milard@linaro.org> wrote:

>>

>>> Hi Bill,

>>> Just sent you a v4 containing my proposal regarding the FSMs and the

>>> text around it.

>>> I still have a few comments:

>>>

>>> First, your FSM mixed events and actions on the transition arrow. I

>>> could not get graphviz to have 2 colors on the same transition (one for

>>> events, one for actions), so I have restricted them to events. The only

>>> action worth mentioning is the event queuing action which is clearly

>>> described by your text (and a reminder in mine). The square node could be a

>>> way to represent it, but I don't like mixing event and action

>>> undistinguished. As long as it is clear and consistent (it seems I have a

>>> need for that these days...), I won't bother you.

>>>

>>

>> I think it's fine this way.  The expiration/schedule call adds detail

>> because a timeout isn't actually delivered until the scheduler returns it

>> to a thread, but I think this is clear enough here.

>>

>>

>

> On V5:

> Maybe the error originated from me (?), but I see that the TO_Delivered

> node is double ringed: Double ringed node would show the start state, so I

> think it shouldn't be double ringed. If the intention was to show the final

> state (though there isn't really one here: a TO event can be reused I

> assume), then the Timer_Expired state should be double ringed as well. But

> as mentionned, as I cannot clearly see any final state here, I would simply

> remove the double ring from the TO_delivered state.

>


Yes, I noticed that too after I sent it.  I'll remove it in v6.



>

>>> Then a few questions:

>>>

>>> 1)My Time-out FSM does not show what happens to a time-out event when an

>>> odp_timer_free() is called while the timeout is in delivered state. I

>>> assume the time-out remains in delivered state until a

>>> odp_event_free/timeout_free() is called manually, but I am not sure (having

>>> 2 different FSM actally clarify this point). You are welcome to update my

>>> FSMs if we should keep them.

>>>

>>

>> Nothing happens to the event since the association between timer and

>> timeout ends as soon as the timer expires. At that point the timer is in

>> the "Timer_Expired" state and the Timeout moves to the "Timeout_Delivered"

>> state (Timeout_Pending in my previous diagram). So they're no longer

>> coupled and operations on one no longer affect the other.

>>

>

> In that case there should be a transition TO_delivered->TO_delivered

> labeled odp_timer_free()

>


I don't think that is necessary. The arrows show relevant/permitted state
transitions. Otherwise we'd have most other ODP APIs doing the same. You
stay in TO_Delivered until one of the APIs shown on the arrows is called.


>

>

>>

>>

>>>

>>> 2)Your text says "Following start, applications may allocate, set,

>>> cancel, and free, timers...". I guess the comma after "timer" is a typo.

>>>

>>

>> Yes, thanks for the correction.

>>

>>

>>>

>>> 3)The last sentence in your text is: "However, upon receiving a timeout

>>> event the application can use the odp_timeout_fresh() API to inquire

>>> whether the timeout event is fresh or stale". What is the definition of a

>>> stale timer?

>>>

>>

>> This was a back reference to the earlier sentence discussing

>> odp_timer_cancel(): "An attempted cancel will fail if the timer is not set

>> or if it has already

>> expired."  However, I recall this was an area that Ola and Petri debated

>> extensively when these were first defined. There is an implicit imprecision

>> in any efficient timer system since independent threads can be setting,

>> resetting, and cancelling timers while they are "in flight". Generally when

>> a timeout event is received it's the responsibility of the application to

>> determine whether that event is meaningful or not based on other

>> information it has (e.g., it's a TCP delayed ACK timer, but we've just

>> received a packet on that connection so I should ignore this event, etc.).

>>  odp_timer_fresh() is a hook that allows the implementation to communicate

>> staleness information if it knows something that the application might not

>> otherwise know, but I expect most applications will just ignore this as a

>> reported "fresh" timer may still not be meaningful from the application's

>> perspective for reasons it knows.

>>

>

> Not sure I understand that. I'll need explanations, and I guess the doc as

> well.

>


I'll try to rework and expand on that a bit but the bottom line is I don't
think that API is particularly well defined or useful.


>

>>

>>>

>>> 4) And I kept the best for the end... :-)

>>> What about the validity of the pointer returned by

>>> odp_timeout_user_ptr() when the timer was set by odp thread A on a queue

>>> belonging to ODP thread B (B != A). Interresting question isn't it? :-).

>>>

>>>

>> The user ptr should really be revised for Tiger Moth to include a length

>> (just as we did for queues) so that it can be part of scheduler

>> prefetching. But as you know Monarch assumes everything is running in the

>> same address space.  Make a note of this (and similar context pointers) as

>> this is something we need to cover in Tiger Moth process discussions.

>>

>

> OK. Pity you are right here as I saw this as a good opportunity to trigger

> some more opinions regarding the the process/thread debate... But you are

> right: the aim is Monarch, and monarch just goes thread, so I cannot stop

> you there :-)

>

> I suggest we take a small HO as soon as possible (ping me when you want)

> so you can explain the thing I don't understand and we can agree on a

> phrasing around it on the fly.

> Sorry, I see a need for V6, but I'll be on your side to make it the last

> one :-)

>

> Christophe.

>

>>

>>

>>

>>> Thanks for reading...

>>>

>>

>>

>> After now reading this I don't think a v6 is needed unless you do.

>>

>>

>>>

>>> Christophe.

>>>

>>> On 12 May 2016 at 14:08, Bill Fischofer <bill.fischofer@linaro.org>

>>> wrote:

>>>

>>>> The green is for Timer related transitions while the blue is for

>>>> Timeout related transitions. This format also seemed to get rid of some of

>>>> the annoying line crossings. The red arrows are the transitions that relate

>>>> to timer expiration and resulting timeout event scheduling, neither of

>>>> which involve Timer APIs. So the colors were supposed to make things easier

>>>> to read.

>>>>

>>>> I'm all for improving clarity, so if you have a better way of

>>>> diagraming this that's great. I believe the diagrams and text go together

>>>> in either case and are intended to be taken together to improve

>>>> understanding.  The goal of each of these sections of the User Guide should

>>>> be to give readers confidence that they understand how to use these APIs in

>>>> writing their own ODP applications, or for ODP implementers to understand

>>>> the semantics they need to provide in their own implementations of them.

>>>>

>>>> On Thu, May 12, 2016 at 3:13 AM, Christophe Milard <

>>>> christophe.milard@linaro.org> wrote:

>>>>

>>>>> Hi,

>>>>>

>>>>> I am still confused with this shared FSM for the 2 distinct objects:

>>>>> The red seems to be actions associated with the transitions (and the timer

>>>>> expiration event is no longer shown on the transition showing the actions).

>>>>> I am not sure what the distinction between the green and blue events are.

>>>>> I propose the following: I will send a complete proposal to you,

>>>>> including both the FSMs and the text. If we still disagree on which one is

>>>>> clearer, we'll let other reviewer decide (you/I would sent a v4 with my

>>>>> stuff in). If no one does react, I am ok to mark your proposal as reviewed

>>>>> because some doc is better than none.

>>>>>

>>>>> So you are sure to  win :-)

>>>>>

>>>>> Christophe.

>>>>>

>>>>> On 11 May 2016 at 01:47, Bill Fischofer <bill.fischofer@linaro.org>

>>>>> wrote:

>>>>>

>>>>>> I looked at them and I liked the added color so reworked my diagram

>>>>>> adding that and posted a v3 for it. I'm not sure trying to separate into

>>>>>> two diagrams really helps but perhaps this reworked diagram is easier to

>>>>>> follow?

>>>>>>

>>>>>> On Tue, May 10, 2016 at 3:45 AM, Christophe Milard <

>>>>>> christophe.milard@linaro.org> wrote:

>>>>>>

>>>>>>> Hi Bill,

>>>>>>>

>>>>>>> Just sent you an RFC with 2 FSMs.

>>>>>>> Had to fight a bit with graphviz. Not finished yet. I will continue

>>>>>>> working on it if you think that makes sense...

>>>>>>> Christophe.

>>>>>>>

>>>>>>> On 9 May 2016 at 17:15, Bill Fischofer <bill.fischofer@linaro.org>

>>>>>>> wrote:

>>>>>>>

>>>>>>>> As we discussed in our call, we can both play around with graphviz

>>>>>>>> and see if a prettier diagram can be constructed that is perhaps clearer,

>>>>>>>> possibly with two separate FSMs.

>>>>>>>>

>>>>>>>> On Mon, May 9, 2016 at 9:39 AM, Christophe Milard <

>>>>>>>> christophe.milard@linaro.org> wrote:

>>>>>>>>

>>>>>>>>>

>>>>>>>>>

>>>>>>>>> On 9 May 2016 at 16:28, Bill Fischofer <bill.fischofer@linaro.org>

>>>>>>>>> wrote:

>>>>>>>>>

>>>>>>>>>>

>>>>>>>>>>

>>>>>>>>>> On Mon, May 9, 2016 at 7:58 AM, Christophe Milard <

>>>>>>>>>> christophe.milard@linaro.org> wrote:

>>>>>>>>>>

>>>>>>>>>>> I am a bit confused by this diagram: It feels to me that timers

>>>>>>>>>>> and timeout have separate lives (even , if of course some dependency

>>>>>>>>>>> exist).

>>>>>>>>>>> From this diagram, it feels like these 2 separate objects

>>>>>>>>>>> (timers and time out events) share states...? do they?

>>>>>>>>>>>

>>>>>>>>>>

>>>>>>>>>> Actually they do, which is what this diagram is trying to

>>>>>>>>>> express. When a timer is set one of the arguments is the timeout event that

>>>>>>>>>> should be associated with it, so it is an error to attempt to free that

>>>>>>>>>> event while it is associated with a set timer (results are undefined if you

>>>>>>>>>> do).  Timers are somewhat unique in this respect.

>>>>>>>>>>

>>>>>>>>>

>>>>>>>>> Sorry, Bill I still don't get it: Aren't you saying here that

>>>>>>>>> these are 2 separate FSMs, but that these 2 separate FSMs are actually

>>>>>>>>> actionned/"triggered" by (partly) the same events? There is nothing wrong

>>>>>>>>> with that... Then they should be represented as 2 separated FSM with the

>>>>>>>>> same event names on the transitions triggered by the same events...

>>>>>>>>>  Or I am very confused...

>>>>>>>>>

>>>>>>>>> Christophe.

>>>>>>>>>

>>>>>>>>>>

>>>>>>>>>>

>>>>>>>>>>> In my head, and from the understanding I had, it feels that the

>>>>>>>>>>> to 4 top states are for timers, whereas time-out events have only 2 states:

>>>>>>>>>>> Unallocated or allocated.

>>>>>>>>>>> It feels you are doing 1 FSM out of two. Maybe , it should be

>>>>>>>>>>> two FSM (keeping the same transition names to show the dependancy.)

>>>>>>>>>>>

>>>>>>>>>>> And I guess there is a typo at "odp_timeout_freee().

>>>>>>>>>>>

>>>>>>>>>>

>>>>>>>>>> Thanks.  I'll fix that in the next version.

>>>>>>>>>>

>>>>>>>>>>

>>>>>>>>>>>

>>>>>>>>>>> Christophe

>>>>>>>>>>>

>>>>>>>>>>> On 7 May 2016 at 18:15, Bill Fischofer <

>>>>>>>>>>> bill.fischofer@linaro.org> wrote:

>>>>>>>>>>>

>>>>>>>>>>>> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

>>>>>>>>>>>> ---

>>>>>>>>>>>>  doc/images/.gitignore       |  1 +

>>>>>>>>>>>>  doc/images/timer_fsm.gv     | 38

>>>>>>>>>>>> ++++++++++++++++++++++++++++++++++++++

>>>>>>>>>>>>  doc/users-guide/Makefile.am |  1 +

>>>>>>>>>>>>  3 files changed, 40 insertions(+)

>>>>>>>>>>>>  create mode 100644 doc/images/timer_fsm.gv

>>>>>>>>>>>>

>>>>>>>>>>>> diff --git a/doc/images/.gitignore b/doc/images/.gitignore

>>>>>>>>>>>> index a19aa75..72cf7ec 100644

>>>>>>>>>>>> --- a/doc/images/.gitignore

>>>>>>>>>>>> +++ b/doc/images/.gitignore

>>>>>>>>>>>> @@ -1,2 +1,3 @@

>>>>>>>>>>>>  resource_management.svg

>>>>>>>>>>>>  pktio_fsm.svg

>>>>>>>>>>>> +timer_fsm.svg

>>>>>>>>>>>> diff --git a/doc/images/timer_fsm.gv b/doc/images/timer_fsm.gv

>>>>>>>>>>>> new file mode 100644

>>>>>>>>>>>> index 0000000..f8cb21a

>>>>>>>>>>>> --- /dev/null

>>>>>>>>>>>> +++ b/doc/images/timer_fsm.gv

>>>>>>>>>>>> @@ -0,0 +1,38 @@

>>>>>>>>>>>> +digraph timer_state_machine {

>>>>>>>>>>>> +       rankdir=LR;

>>>>>>>>>>>> +       size="12,20";

>>>>>>>>>>>> +       node [fontsize=28];

>>>>>>>>>>>> +       edge [fontsize=28];

>>>>>>>>>>>> +       node [shape=doublecircle]; Timer_Unalloc

>>>>>>>>>>>> +                                  Timeout_Unalloc

>>>>>>>>>>>> +                                  Timeout_Delivered;

>>>>>>>>>>>> +        node [shape=rectangle]; Timeout_Queued;

>>>>>>>>>>>> +       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 -> Timeout_Queued [label="=>odp_queue_enq()"];

>>>>>>>>>>>> +       Timeout_Queued -> 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];

>>>>>>>>>>>> +}

>>>>>>>>>>>> diff --git a/doc/users-guide/Makefile.am

>>>>>>>>>>>> b/doc/users-guide/Makefile.am

>>>>>>>>>>>> index 74caa96..6bb0131 100644

>>>>>>>>>>>> --- a/doc/users-guide/Makefile.am

>>>>>>>>>>>> +++ b/doc/users-guide/Makefile.am

>>>>>>>>>>>> @@ -30,6 +30,7 @@ IMAGES =

>>>>>>>>>>>> $(top_srcdir)/doc/images/overview.svg \

>>>>>>>>>>>>          $(top_srcdir)/doc/images/release_git.svg \

>>>>>>>>>>>>          $(top_srcdir)/doc/images/segment.svg \

>>>>>>>>>>>>          $(top_srcdir)/doc/images/simple_release_git.svg \

>>>>>>>>>>>> +        $(top_srcdir)/doc/images/timer_fsm.svg \

>>>>>>>>>>>>          $(top_srcdir)/doc/images/tm_hierarchy.svg \

>>>>>>>>>>>>          $(top_srcdir)/doc/images/tm_node.svg

>>>>>>>>>>>>

>>>>>>>>>>>> --

>>>>>>>>>>>> 2.5.0

>>>>>>>>>>>>

>>>>>>>>>>>> _______________________________________________

>>>>>>>>>>>> lng-odp mailing list

>>>>>>>>>>>> lng-odp@lists.linaro.org

>>>>>>>>>>>> https://lists.linaro.org/mailman/listinfo/lng-odp

>>>>>>>>>>>>

>>>>>>>>>>>

>>>>>>>>>>>

>>>>>>>>>>

>>>>>>>>>

>>>>>>>>

>>>>>>>

>>>>>>

>>>>>

>>>>

>>>

>>

>
diff mbox

Patch

diff --git a/doc/images/.gitignore b/doc/images/.gitignore
index a19aa75..72cf7ec 100644
--- a/doc/images/.gitignore
+++ b/doc/images/.gitignore
@@ -1,2 +1,3 @@ 
 resource_management.svg
 pktio_fsm.svg
+timer_fsm.svg
diff --git a/doc/images/timer_fsm.gv b/doc/images/timer_fsm.gv
new file mode 100644
index 0000000..f8cb21a
--- /dev/null
+++ b/doc/images/timer_fsm.gv
@@ -0,0 +1,38 @@ 
+digraph timer_state_machine {
+	rankdir=LR;
+	size="12,20";
+	node [fontsize=28];
+	edge [fontsize=28];
+	node [shape=doublecircle]; Timer_Unalloc
+	     			   Timeout_Unalloc
+	     			   Timeout_Delivered;
+        node [shape=rectangle]; Timeout_Queued;
+	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 -> Timeout_Queued [label="=>odp_queue_enq()"];
+	Timeout_Queued -> 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];
+}
diff --git a/doc/users-guide/Makefile.am b/doc/users-guide/Makefile.am
index 74caa96..6bb0131 100644
--- a/doc/users-guide/Makefile.am
+++ b/doc/users-guide/Makefile.am
@@ -30,6 +30,7 @@  IMAGES = $(top_srcdir)/doc/images/overview.svg \
 	 $(top_srcdir)/doc/images/release_git.svg \
 	 $(top_srcdir)/doc/images/segment.svg \
 	 $(top_srcdir)/doc/images/simple_release_git.svg \
+	 $(top_srcdir)/doc/images/timer_fsm.svg \
 	 $(top_srcdir)/doc/images/tm_hierarchy.svg \
 	 $(top_srcdir)/doc/images/tm_node.svg