[1/2] api: scheduler: atomic and ordered definitions

Improved documentation and definition of atomic and ordered
queue synchronisation.

Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>

---
This is the ordered queue definition (in patch format) promised
in the call yesterday.
---
 include/odp/api/queue.h    | 21 +++++++++++++++++++--
 include/odp/api/schedule.h | 15 ++++++++++++---
 2 files changed, 31 insertions(+), 5 deletions(-)

On 17 February 2015 at 11:49, Petri Savolainen
<petri.savolainen@linaro.org> wrote:
> Improved documentation and definition of atomic and ordered
> queue synchronisation.
>
> Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>
>
> ---
> This is the ordered queue definition (in patch format) promised
> in the call yesterday.
> ---
>  include/odp/api/queue.h    | 21 +++++++++++++++++++--
>  include/odp/api/schedule.h | 15 ++++++++++++---
>  2 files changed, 31 insertions(+), 5 deletions(-)
>
> diff --git a/include/odp/api/queue.h b/include/odp/api/queue.h
> index 9519edf..1d67a3c 100644
> --- a/include/odp/api/queue.h
> +++ b/include/odp/api/queue.h
> @@ -108,12 +108,29 @@ extern "C" {
>
>  /**
>   * @def ODP_SCHED_SYNC_ATOMIC
> - * Atomic queue
> + * Atomic queue synchronisation
> + *
> + * Events from an atomic queue can be scheduled only to a single thread at a
I question the use of "schedule to", is this really proper language?
Events are scheduled but the events selected are then 'dispatched' to a thread.
Suggestion: "Events from an atomic queue can be dispatched only to a
single thread at a time, this  avoids ".

> + * time. This quarantees atomic access to the queue context and maintains event
Spelling error, should be "guarantees".
Suggestion: "Atomic scheduling provides mutual exclusion and atomic
access to the associated queue context and maintains event ordering.
This enables the user to avoid synchronization and event reordering in
software."

> + * ordering, which helps the user to avoid SW locking.
> + *
> + * The atomic queue is dedicated to the thread until it requests another event
Suggestion: "The selected atomic queue is dedicated to the thread
until the thread requests another event from the scheduler (which
implicitly releases the queue) or the thread calls
odp_schedule_release_atomic() (which allows the scheduler to release
the queue immediately)."

> + * from the scheduler (implicitly requests to release the current queue) or
> + * calls odp_schedule_release_atomic(), which hints the scheduler to release the
> + * queue early.
>   */
>
>  /**
>   * @def ODP_SCHED_SYNC_ORDERED
> - * Ordered queue
> + * Ordered queue synchronisation
> + *
> + * Events from an ordered queue can be scheduled to multiple threads for
> + * parallel processing.
The phrase "parallel processing" indicates that *one* piece of work is
being processed in parallel by multiple processors. But we are not
talking about one event being processed in parallel, it is different
events (from the same queue) that may be processed concurrently (by
different processors). It's borderline but I suggest we avoid the use
of "parallel" in order to minimize the risk for confusion.

Suggestion: "Events from an ordered queue can be scheduled and
dispatched to multiple threads for concurrent processing."

> The original enqueue order of the source queue is
Isn't it the event dequeue order of the source queue that is being
maintained? Yes this might be the same as the enqueue order to the
source queue (since queues supposedly are FIFO) but if enqueueing is
done from another ordered queue, threads may call odp_queue_enq() in a
different order and the events will be reordered. It is this enqueue
order the software sees but it is not this order ordered queues are
maintaining.

Suggestion: "The (dequeue) event order of the source queue is
maintained when events are enqueued to their destination queue(s)
(before another call to the scheduler)." Maybe drop the parentheses
from the last phrase.

> + * maintained when events are enqueued to their destination queue(s) before
> + * another schedule call. Events from the same (source) queue appear in their
> + * original order when dequeued from a destination queue. The destination
> + * queue type (POLL/SCHED) or synchronisation (NONE/ATOMIC/ORDERED) is not
> + * limited.
What about packet input and packet output queues?

Suggestion: "The destination queue type (polled, scheduled or packet
input/output) or synchronization model (none, ordered or atomic) is of
no concern."

>   */
>
>  /**
> diff --git a/include/odp/api/schedule.h b/include/odp/api/schedule.h
> index 5c08357..378ca19 100644
> --- a/include/odp/api/schedule.h
> +++ b/include/odp/api/schedule.h
> @@ -93,9 +93,9 @@ int odp_schedule_multi(odp_queue_t *from, uint64_t wait, odp_event_t events[],
>   * Pause scheduling
>   *
>   * Pause global scheduling for this thread. After this call, all schedule calls
> - * will return only locally reserved buffers (if any). User can exit the
> + * will return only locally reserved events (if any). User can exit the
"reserved"?
Reserved items can normally be unreserved but I don't think this is
the case here.
Better to use "pre-scheduled events" which must be consumed by
(dispatched to) the thread in question.

>   * schedule loop only after the schedule function indicates that there's no more
> - * buffers (no more locally reserved buffers).
> + * events (no more locally reserved events).
>   *
>   * Must be used with odp_schedule() and odp_schedule_multi() before exiting (or
>   * stalling) the schedule loop.
> @@ -111,7 +111,16 @@ void odp_schedule_pause(void);
>  void odp_schedule_resume(void);
>
>  /**
> - * Release currently hold atomic context
> + * Release the current atomic context
> + *
> + * This call is valid only for source queues with atomic synchronisation. It
English or American spelling? "s" vs. "z".

> + * hints the scheduler that the user has completed processing of the critical
> + * section (which depends on the atomic synchronisation). The scheduler is now
Same here.

> + * allowed to schedule another event(s) from the same queue to another thread.
Suggestion: "The scheduler is now allowed to schedule and dispatch
further events from the same queue to some other thread".

> + *
> + * Early atomic context release may increase parallelism and thus system
> + * performance, but user needs to design carefully the split into critical vs.
"design carefully" => "carefully design"

> + * non-critical sections.
>   */
>  void odp_schedule_release_atomic(void);
>
> --
> 2.3.0
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp

On 17 February 2015 at 15:49, Ola Liljedahl <ola.liljedahl@linaro.org> wrote:
> On 17 February 2015 at 11:49, Petri Savolainen
> <petri.savolainen@linaro.org> wrote:
>> Improved documentation and definition of atomic and ordered
>> queue synchronisation.
>>
>> Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>
>>
>> ---
>> This is the ordered queue definition (in patch format) promised
>> in the call yesterday.
>> ---
>>  include/odp/api/queue.h    | 21 +++++++++++++++++++--
>>  include/odp/api/schedule.h | 15 ++++++++++++---
>>  2 files changed, 31 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/odp/api/queue.h b/include/odp/api/queue.h
>> index 9519edf..1d67a3c 100644
>> --- a/include/odp/api/queue.h
>> +++ b/include/odp/api/queue.h
>> @@ -108,12 +108,29 @@ extern "C" {
>>
>>  /**
>>   * @def ODP_SCHED_SYNC_ATOMIC
>> - * Atomic queue
>> + * Atomic queue synchronisation
>> + *
>> + * Events from an atomic queue can be scheduled only to a single thread at a
> I question the use of "schedule to", is this really proper language?
> Events are scheduled but the events selected are then 'dispatched' to a thread.
> Suggestion: "Events from an atomic queue can be dispatched only to a
> single thread at a time, this  avoids ".
Should have been: "Events from an atomic queue can be dispatched only
to a single thread at a time."
The "avoid" stuff is below.

>
>> + * time. This quarantees atomic access to the queue context and maintains event
> Spelling error, should be "guarantees".
> Suggestion: "Atomic scheduling provides mutual exclusion and atomic
> access to the associated queue context and maintains event ordering.
> This enables the user to avoid synchronization and event reordering in
> software."
>
>> + * ordering, which helps the user to avoid SW locking.
>> + *
>> + * The atomic queue is dedicated to the thread until it requests another event
> Suggestion: "The selected atomic queue is dedicated to the thread
> until the thread requests another event from the scheduler (which
> implicitly releases the queue) or the thread calls
> odp_schedule_release_atomic() (which allows the scheduler to release
> the queue immediately)."
>
>> + * from the scheduler (implicitly requests to release the current queue) or
>> + * calls odp_schedule_release_atomic(), which hints the scheduler to release the
>> + * queue early.
>>   */
>>
>>  /**
>>   * @def ODP_SCHED_SYNC_ORDERED
>> - * Ordered queue
>> + * Ordered queue synchronisation
>> + *
>> + * Events from an ordered queue can be scheduled to multiple threads for
>> + * parallel processing.
> The phrase "parallel processing" indicates that *one* piece of work is
> being processed in parallel by multiple processors. But we are not
> talking about one event being processed in parallel, it is different
> events (from the same queue) that may be processed concurrently (by
> different processors). It's borderline but I suggest we avoid the use
> of "parallel" in order to minimize the risk for confusion.
>
> Suggestion: "Events from an ordered queue can be scheduled and
> dispatched to multiple threads for concurrent processing."
>
>> The original enqueue order of the source queue is
> Isn't it the event dequeue order of the source queue that is being
> maintained? Yes this might be the same as the enqueue order to the
> source queue (since queues supposedly are FIFO) but if enqueueing is
> done from another ordered queue, threads may call odp_queue_enq() in a
> different order and the events will be reordered. It is this enqueue
> order the software sees but it is not this order ordered queues are
> maintaining.
>
> Suggestion: "The (dequeue) event order of the source queue is
> maintained when events are enqueued to their destination queue(s)
> (before another call to the scheduler)." Maybe drop the parentheses
> from the last phrase.
>
>> + * maintained when events are enqueued to their destination queue(s) before
>> + * another schedule call. Events from the same (source) queue appear in their
>> + * original order when dequeued from a destination queue. The destination
>> + * queue type (POLL/SCHED) or synchronisation (NONE/ATOMIC/ORDERED) is not
>> + * limited.
> What about packet input and packet output queues?
>
> Suggestion: "The destination queue type (polled, scheduled or packet
> input/output) or synchronization model (none, ordered or atomic) is of
> no concern."
>
>>   */
>>
>>  /**
>> diff --git a/include/odp/api/schedule.h b/include/odp/api/schedule.h
>> index 5c08357..378ca19 100644
>> --- a/include/odp/api/schedule.h
>> +++ b/include/odp/api/schedule.h
>> @@ -93,9 +93,9 @@ int odp_schedule_multi(odp_queue_t *from, uint64_t wait, odp_event_t events[],
>>   * Pause scheduling
>>   *
>>   * Pause global scheduling for this thread. After this call, all schedule calls
>> - * will return only locally reserved buffers (if any). User can exit the
>> + * will return only locally reserved events (if any). User can exit the
> "reserved"?
> Reserved items can normally be unreserved but I don't think this is
> the case here.
> Better to use "pre-scheduled events" which must be consumed by
> (dispatched to) the thread in question.
>
>>   * schedule loop only after the schedule function indicates that there's no more
>> - * buffers (no more locally reserved buffers).
>> + * events (no more locally reserved events).
>>   *
>>   * Must be used with odp_schedule() and odp_schedule_multi() before exiting (or
>>   * stalling) the schedule loop.
>> @@ -111,7 +111,16 @@ void odp_schedule_pause(void);
>>  void odp_schedule_resume(void);
>>
>>  /**
>> - * Release currently hold atomic context
>> + * Release the current atomic context
>> + *
>> + * This call is valid only for source queues with atomic synchronisation. It
> English or American spelling? "s" vs. "z".
>
>> + * hints the scheduler that the user has completed processing of the critical
>> + * section (which depends on the atomic synchronisation). The scheduler is now
> Same here.
>
>> + * allowed to schedule another event(s) from the same queue to another thread.
> Suggestion: "The scheduler is now allowed to schedule and dispatch
> further events from the same queue to some other thread".
>
>> + *
>> + * Early atomic context release may increase parallelism and thus system
>> + * performance, but user needs to design carefully the split into critical vs.
> "design carefully" => "carefully design"
>
>> + * non-critical sections.
>>   */
>>  void odp_schedule_release_atomic(void);
>>
>> --
>> 2.3.0
>>
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/lng-odp

+1 for Ola's suggested wording, except the last.  The wording "to design
carefully" is grammatically correct English.  The alternative "to carefully
design" although common is technically a split infinitive which is
considered incorrect.

Bill

On Tue, Feb 17, 2015 at 8:51 AM, Ola Liljedahl <ola.liljedahl@linaro.org>
wrote:

> On 17 February 2015 at 15:49, Ola Liljedahl <ola.liljedahl@linaro.org>
> wrote:
> > On 17 February 2015 at 11:49, Petri Savolainen
> > <petri.savolainen@linaro.org> wrote:
> >> Improved documentation and definition of atomic and ordered
> >> queue synchronisation.
> >>
> >> Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>
> >>
> >> ---
> >> This is the ordered queue definition (in patch format) promised
> >> in the call yesterday.
> >> ---
> >>  include/odp/api/queue.h    | 21 +++++++++++++++++++--
> >>  include/odp/api/schedule.h | 15 ++++++++++++---
> >>  2 files changed, 31 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/include/odp/api/queue.h b/include/odp/api/queue.h
> >> index 9519edf..1d67a3c 100644
> >> --- a/include/odp/api/queue.h
> >> +++ b/include/odp/api/queue.h
> >> @@ -108,12 +108,29 @@ extern "C" {
> >>
> >>  /**
> >>   * @def ODP_SCHED_SYNC_ATOMIC
> >> - * Atomic queue
> >> + * Atomic queue synchronisation
> >> + *
> >> + * Events from an atomic queue can be scheduled only to a single
> thread at a
> > I question the use of "schedule to", is this really proper language?
> > Events are scheduled but the events selected are then 'dispatched' to a
> thread.
> > Suggestion: "Events from an atomic queue can be dispatched only to a
> > single thread at a time, this  avoids ".
> Should have been: "Events from an atomic queue can be dispatched only
> to a single thread at a time."
> The "avoid" stuff is below.
>
> >
> >> + * time. This quarantees atomic access to the queue context and
> maintains event
> > Spelling error, should be "guarantees".
> > Suggestion: "Atomic scheduling provides mutual exclusion and atomic
> > access to the associated queue context and maintains event ordering.
> > This enables the user to avoid synchronization and event reordering in
> > software."
> >
> >> + * ordering, which helps the user to avoid SW locking.
> >> + *
> >> + * The atomic queue is dedicated to the thread until it requests
> another event
> > Suggestion: "The selected atomic queue is dedicated to the thread
> > until the thread requests another event from the scheduler (which
> > implicitly releases the queue) or the thread calls
> > odp_schedule_release_atomic() (which allows the scheduler to release
> > the queue immediately)."
> >
> >> + * from the scheduler (implicitly requests to release the current
> queue) or
> >> + * calls odp_schedule_release_atomic(), which hints the scheduler to
> release the
> >> + * queue early.
> >>   */
> >>
> >>  /**
> >>   * @def ODP_SCHED_SYNC_ORDERED
> >> - * Ordered queue
> >> + * Ordered queue synchronisation
> >> + *
> >> + * Events from an ordered queue can be scheduled to multiple threads
> for
> >> + * parallel processing.
> > The phrase "parallel processing" indicates that *one* piece of work is
> > being processed in parallel by multiple processors. But we are not
> > talking about one event being processed in parallel, it is different
> > events (from the same queue) that may be processed concurrently (by
> > different processors). It's borderline but I suggest we avoid the use
> > of "parallel" in order to minimize the risk for confusion.
> >
> > Suggestion: "Events from an ordered queue can be scheduled and
> > dispatched to multiple threads for concurrent processing."
> >
> >> The original enqueue order of the source queue is
> > Isn't it the event dequeue order of the source queue that is being
> > maintained? Yes this might be the same as the enqueue order to the
> > source queue (since queues supposedly are FIFO) but if enqueueing is
> > done from another ordered queue, threads may call odp_queue_enq() in a
> > different order and the events will be reordered. It is this enqueue
> > order the software sees but it is not this order ordered queues are
> > maintaining.
> >
> > Suggestion: "The (dequeue) event order of the source queue is
> > maintained when events are enqueued to their destination queue(s)
> > (before another call to the scheduler)." Maybe drop the parentheses
> > from the last phrase.
> >
> >> + * maintained when events are enqueued to their destination queue(s)
> before
> >> + * another schedule call. Events from the same (source) queue appear
> in their
> >> + * original order when dequeued from a destination queue. The
> destination
> >> + * queue type (POLL/SCHED) or synchronisation (NONE/ATOMIC/ORDERED) is
> not
> >> + * limited.
> > What about packet input and packet output queues?
> >
> > Suggestion: "The destination queue type (polled, scheduled or packet
> > input/output) or synchronization model (none, ordered or atomic) is of
> > no concern."
> >
> >>   */
> >>
> >>  /**
> >> diff --git a/include/odp/api/schedule.h b/include/odp/api/schedule.h
> >> index 5c08357..378ca19 100644
> >> --- a/include/odp/api/schedule.h
> >> +++ b/include/odp/api/schedule.h
> >> @@ -93,9 +93,9 @@ int odp_schedule_multi(odp_queue_t *from, uint64_t
> wait, odp_event_t events[],
> >>   * Pause scheduling
> >>   *
> >>   * Pause global scheduling for this thread. After this call, all
> schedule calls
> >> - * will return only locally reserved buffers (if any). User can exit
> the
> >> + * will return only locally reserved events (if any). User can exit the
> > "reserved"?
> > Reserved items can normally be unreserved but I don't think this is
> > the case here.
> > Better to use "pre-scheduled events" which must be consumed by
> > (dispatched to) the thread in question.
> >
> >>   * schedule loop only after the schedule function indicates that
> there's no more
> >> - * buffers (no more locally reserved buffers).
> >> + * events (no more locally reserved events).
> >>   *
> >>   * Must be used with odp_schedule() and odp_schedule_multi() before
> exiting (or
> >>   * stalling) the schedule loop.
> >> @@ -111,7 +111,16 @@ void odp_schedule_pause(void);
> >>  void odp_schedule_resume(void);
> >>
> >>  /**
> >> - * Release currently hold atomic context
> >> + * Release the current atomic context
> >> + *
> >> + * This call is valid only for source queues with atomic
> synchronisation. It
> > English or American spelling? "s" vs. "z".
> >
> >> + * hints the scheduler that the user has completed processing of the
> critical
> >> + * section (which depends on the atomic synchronisation). The
> scheduler is now
> > Same here.
> >
> >> + * allowed to schedule another event(s) from the same queue to another
> thread.
> > Suggestion: "The scheduler is now allowed to schedule and dispatch
> > further events from the same queue to some other thread".
> >
> >> + *
> >> + * Early atomic context release may increase parallelism and thus
> system
> >> + * performance, but user needs to design carefully the split into
> critical vs.
> > "design carefully" => "carefully design"
> >
> >> + * non-critical sections.
> >>   */
> >>  void odp_schedule_release_atomic(void);
> >>
> >> --
> >> 2.3.0
> >>
> >>
> >> _______________________________________________
> >> lng-odp mailing list
> >> lng-odp@lists.linaro.org
> >> http://lists.linaro.org/mailman/listinfo/lng-odp
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>