[v2,10/11] eventdev: RFC clarify comments on scheduling types

Message ID 20240119174346.108905-11-bruce.richardson@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: Jerin Jacob
Headers
Series improve eventdev API specification/documentation |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Bruce Richardson Jan. 19, 2024, 5:43 p.m. UTC
  The description of ordered and atomic scheduling given in the eventdev
doxygen documentation was not always clear. Try and simplify this so
that it is clearer for the end-user of the application

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---

NOTE TO REVIEWERS:
I've updated this based on my understanding of what these scheduling
types are meant to do. It matches my understanding of the support
offered by our Intel DLB2 driver, as well as the SW eventdev, and I
believe the DSW eventdev too. If it does not match the behaviour of
other eventdevs, let's have a discussion to see if we can reach a good
definition of the behaviour that is common.
---
 lib/eventdev/rte_eventdev.h | 47 ++++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 22 deletions(-)
  

Comments

Mattias Rönnblom Jan. 23, 2024, 4:19 p.m. UTC | #1
On 2024-01-19 18:43, Bruce Richardson wrote:
> The description of ordered and atomic scheduling given in the eventdev
> doxygen documentation was not always clear. Try and simplify this so
> that it is clearer for the end-user of the application
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> 
> NOTE TO REVIEWERS:
> I've updated this based on my understanding of what these scheduling
> types are meant to do. It matches my understanding of the support
> offered by our Intel DLB2 driver, as well as the SW eventdev, and I
> believe the DSW eventdev too. If it does not match the behaviour of
> other eventdevs, let's have a discussion to see if we can reach a good
> definition of the behaviour that is common.
> ---
>   lib/eventdev/rte_eventdev.h | 47 ++++++++++++++++++++-----------------
>   1 file changed, 25 insertions(+), 22 deletions(-)
> 
> diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
> index 2c6576e921..cb13602ffb 100644
> --- a/lib/eventdev/rte_eventdev.h
> +++ b/lib/eventdev/rte_eventdev.h
> @@ -1313,26 +1313,24 @@ struct rte_event_vector {
>   #define RTE_SCHED_TYPE_ORDERED          0
>   /**< Ordered scheduling
>    *
> - * Events from an ordered flow of an event queue can be scheduled to multiple
> + * Events from an ordered event queue can be scheduled to multiple

What is the rationale for this change?

An implementation that impose a total order on all events on a 
particular ordered queue will still adhere to the current, more relaxed, 
per-flow ordering semantics.

An application wanting a total order would just set the flow id to 0 on 
all events destined that queue, and it would work on all event devices.

Why don't you just put a note in the DLB driver saying "btw it's total 
order", so any application where per-flow ordering is crucial for 
performance (i.e., where the potentially needless head-of-line blocking 
is an issue) can use multiple queues when running with the DLB.

In the API as-written, the app is free to express more relaxed ordering 
requirements (i.e., to have multiple flows) and it's up to the event 
device to figure out if it's in a position where it can translate this 
to lower latency.

>    * ports for concurrent processing while maintaining the original event order.

Maybe it's worth mentioning what is the original event order. "(i.e., 
the order in which the events were enqueued to the queue)". Especially 
since one like to specify what ordering guarantees one have of events 
enqueued to the same queue on different ports and by different lcores).

I don't know where that information should go though, since it's 
relevant for both atomic and ordered-type queues.

>    * This scheme enables the user to achieve high single flow throughput by
> - * avoiding SW synchronization for ordering between ports which bound to cores.
> - *
> - * The source flow ordering from an event queue is maintained when events are
> - * enqueued to their destination queue within the same ordered flow context.
> - * An event port holds the context until application call
> - * rte_event_dequeue_burst() from the same port, which implicitly releases
> - * the context.
> - * User may allow the scheduler to release the context earlier than that
> - * by invoking rte_event_enqueue_burst() with RTE_EVENT_OP_RELEASE operation.
> - *
> - * Events from the source queue appear in their original order when dequeued
> - * from a destination queue.
> - * Event ordering is based on the received event(s), but also other
> - * (newly allocated or stored) events are ordered when enqueued within the same
> - * ordered context. Events not enqueued (e.g. released or stored) within the
> - * context are  considered missing from reordering and are skipped at this time
> - * (but can be ordered again within another context).
> + * avoiding SW synchronization for ordering between ports which are polled by
> + * different cores.
> + *
> + * As events are scheduled to ports/cores, the original event order from the
> + * source event queue is recorded internally in the scheduler. As events are
> + * returned (via FORWARD type enqueue) to the scheduler, the original event
> + * order is restored before the events are enqueued into their new destination
> + * queue.

Delete the first sentence on implementation.

"As events are re-enqueued to the next queue (with the op field set to 
RTE_EVENT_OP_FORWARD), the event device restores the original event 
order before the events arrive on the destination queue."

> + *
> + * Any events not forwarded, ie. dropped explicitly via RELEASE or implicitly
> + * released by the next dequeue from a port, are skipped by the reordering
> + * stage and do not affect the reordering of returned events.
> + *
> + * The ordering behaviour of NEW events with respect to FORWARD events is
> + * undefined and implementation dependent.

For some reason I find this a little vague. "NEW and FORWARD events 
enqueued to a queue are not ordered in relation to each other (even if 
the flow id is the same)."

I think I agree that NEW shouldn't be ordered vis-a-vi FORWARD, but 
maybe one should say that an event device should avoid excessive 
reordering NEW and FORWARD events.

I think it would also be helpful to address port-to-port ordering 
guarantees (or a lack thereof).

"Events enqueued on one port are not ordered in relation to events 
enqueued on some other port."

Or are they? Not in DSW, at least, and I'm not sure I see a use case for 
such a guarantee, but it's a little counter-intuitive to have them 
potentially re-shuffled.

(This is also relevant for atomic queues.)

>    *
>    * @see rte_event_queue_setup(), rte_event_dequeue_burst(), RTE_EVENT_OP_RELEASE
>    */
> @@ -1340,18 +1338,23 @@ struct rte_event_vector {
>   #define RTE_SCHED_TYPE_ATOMIC           1
>   /**< Atomic scheduling
>    *
> - * Events from an atomic flow of an event queue can be scheduled only to a
> + * Events from an atomic flow, identified by @ref rte_event.flow_id,

A flow is identified by the combination of queue_id and flow_id, so if 
you reference one you should also reference the other.

> + * of an event queue can be scheduled only to a
>    * single port at a time. The port is guaranteed to have exclusive (atomic)
>    * access to the associated flow context, which enables the user to avoid SW
>    * synchronization. Atomic flows also help to maintain event ordering

"help" here needs to go, I think. It sounds like a best-effort affair. 
The atomic queue ordering guarantees (or the lack thereof) should be 
spelled out.

"Event order in an atomic flow is maintained."

> - * since only one port at a time can process events from a flow of an
> + * since only one port at a time can process events from each flow of an
>    * event queue.

Yes, and *but also since* the event device is not reshuffling events 
enqueued to an atomic queue. And that's more complicated than just 
something that falls out of atomicity, especially if you assume that 
FORWARD type enqueues are not ordered with other FORWARD type enqueues 
on a different port.

>    *
> - * The atomic queue synchronization context is dedicated to the port until
> + * The atomic queue synchronization context for a flow is dedicated to the port until

What is an "atomic queue synchronization context" (except for something 
that makes for long sentences).

How about:
"The atomic flow is locked to the port until /../"

You could also used the word "bound" instead of "locked".

>    * application call rte_event_dequeue_burst() from the same port,
>    * which implicitly releases the context. User may allow the scheduler to
>    * release the context earlier than that by invoking rte_event_enqueue_burst()
> - * with RTE_EVENT_OP_RELEASE operation.
> + * with RTE_EVENT_OP_RELEASE operation for each event from that flow. The context
> + * is only released once the last event from the flow, outstanding on the port,
> + * is released. So long as there is one event from an atomic flow scheduled to
> + * a port/core (including any events in the port's dequeue queue, not yet read
> + * by the application), that port will hold the synchronization context.

In case you like the "atomic flow locked/bound to port", this part would 
also need updating.

Maybe here is a good place to add a note on memory ordering and event 
ordering.

"Any memory stores done as a part of event processing will be globally 
visible before the next event in the same atomic flow is dequeued on a 
different lcore."

I.e., enqueue includes write barrier before the event can be seen.

One should probably mentioned a rmb in dequeue as well.

>    *
>    * @see rte_event_queue_setup(), rte_event_dequeue_burst(), RTE_EVENT_OP_RELEASE
>    */
  
Bruce Richardson Jan. 24, 2024, 11:21 a.m. UTC | #2
On Tue, Jan 23, 2024 at 05:19:18PM +0100, Mattias Rönnblom wrote:
> On 2024-01-19 18:43, Bruce Richardson wrote:
> > The description of ordered and atomic scheduling given in the eventdev
> > doxygen documentation was not always clear. Try and simplify this so
> > that it is clearer for the end-user of the application
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> > 
> > NOTE TO REVIEWERS:
> > I've updated this based on my understanding of what these scheduling
> > types are meant to do. It matches my understanding of the support
> > offered by our Intel DLB2 driver, as well as the SW eventdev, and I
> > believe the DSW eventdev too. If it does not match the behaviour of
> > other eventdevs, let's have a discussion to see if we can reach a good
> > definition of the behaviour that is common.
> > ---
> >   lib/eventdev/rte_eventdev.h | 47 ++++++++++++++++++++-----------------
> >   1 file changed, 25 insertions(+), 22 deletions(-)
> > 
> > diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
> > index 2c6576e921..cb13602ffb 100644
> > --- a/lib/eventdev/rte_eventdev.h
> > +++ b/lib/eventdev/rte_eventdev.h
> > @@ -1313,26 +1313,24 @@ struct rte_event_vector {
> >   #define RTE_SCHED_TYPE_ORDERED          0
> >   /**< Ordered scheduling
> >    *
> > - * Events from an ordered flow of an event queue can be scheduled to multiple
> > + * Events from an ordered event queue can be scheduled to multiple
> 
> What is the rationale for this change?
> 
> An implementation that impose a total order on all events on a particular
> ordered queue will still adhere to the current, more relaxed, per-flow
> ordering semantics.
> 
> An application wanting a total order would just set the flow id to 0 on all
> events destined that queue, and it would work on all event devices.
> 
> Why don't you just put a note in the DLB driver saying "btw it's total
> order", so any application where per-flow ordering is crucial for
> performance (i.e., where the potentially needless head-of-line blocking is
> an issue) can use multiple queues when running with the DLB.
> 
> In the API as-written, the app is free to express more relaxed ordering
> requirements (i.e., to have multiple flows) and it's up to the event device
> to figure out if it's in a position where it can translate this to lower
> latency.
> 

Yes, you are right. I'll roll-back or rework this change in V3. Keep it
documented that flow-ordering is guaranteed, but note that some
implementations may use total ordering to achieve that.

> >    * ports for concurrent processing while maintaining the original event order.
> 
> Maybe it's worth mentioning what is the original event order. "(i.e., the
> order in which the events were enqueued to the queue)". Especially since one
> like to specify what ordering guarantees one have of events enqueued to the
> same queue on different ports and by different lcores).
> 
> I don't know where that information should go though, since it's relevant
> for both atomic and ordered-type queues.
> 

It's probably more relevant for ordered, but I'll try and see where it's
best to go.

> >    * This scheme enables the user to achieve high single flow throughput by
> > - * avoiding SW synchronization for ordering between ports which bound to cores.
> > - *
> > - * The source flow ordering from an event queue is maintained when events are
> > - * enqueued to their destination queue within the same ordered flow context.
> > - * An event port holds the context until application call
> > - * rte_event_dequeue_burst() from the same port, which implicitly releases
> > - * the context.
> > - * User may allow the scheduler to release the context earlier than that
> > - * by invoking rte_event_enqueue_burst() with RTE_EVENT_OP_RELEASE operation.
> > - *
> > - * Events from the source queue appear in their original order when dequeued
> > - * from a destination queue.
> > - * Event ordering is based on the received event(s), but also other
> > - * (newly allocated or stored) events are ordered when enqueued within the same
> > - * ordered context. Events not enqueued (e.g. released or stored) within the
> > - * context are  considered missing from reordering and are skipped at this time
> > - * (but can be ordered again within another context).
> > + * avoiding SW synchronization for ordering between ports which are polled by
> > + * different cores.
> > + *
> > + * As events are scheduled to ports/cores, the original event order from the
> > + * source event queue is recorded internally in the scheduler. As events are
> > + * returned (via FORWARD type enqueue) to the scheduler, the original event
> > + * order is restored before the events are enqueued into their new destination
> > + * queue.
> 
> Delete the first sentence on implementation.
> 
> "As events are re-enqueued to the next queue (with the op field set to
> RTE_EVENT_OP_FORWARD), the event device restores the original event order
> before the events arrive on the destination queue."
> 
> > + *
> > + * Any events not forwarded, ie. dropped explicitly via RELEASE or implicitly
> > + * released by the next dequeue from a port, are skipped by the reordering
> > + * stage and do not affect the reordering of returned events.
> > + *
> > + * The ordering behaviour of NEW events with respect to FORWARD events is
> > + * undefined and implementation dependent.
> 
> For some reason I find this a little vague. "NEW and FORWARD events enqueued
> to a queue are not ordered in relation to each other (even if the flow id is
> the same)."
> 
> I think I agree that NEW shouldn't be ordered vis-a-vi FORWARD, but maybe
> one should say that an event device should avoid excessive reordering NEW
> and FORWARD events.
> 
> I think it would also be helpful to address port-to-port ordering guarantees
> (or a lack thereof).
> 
> "Events enqueued on one port are not ordered in relation to events enqueued
> on some other port."
> 
> Or are they? Not in DSW, at least, and I'm not sure I see a use case for
> such a guarantee, but it's a little counter-intuitive to have them
> potentially re-shuffled.
> 
> (This is also relevant for atomic queues.)
> 

Ack.

> >    *
> >    * @see rte_event_queue_setup(), rte_event_dequeue_burst(), RTE_EVENT_OP_RELEASE
> >    */
> > @@ -1340,18 +1338,23 @@ struct rte_event_vector {
> >   #define RTE_SCHED_TYPE_ATOMIC           1
> >   /**< Atomic scheduling
> >    *
> > - * Events from an atomic flow of an event queue can be scheduled only to a
> > + * Events from an atomic flow, identified by @ref rte_event.flow_id,
> 
> A flow is identified by the combination of queue_id and flow_id, so if you
> reference one you should also reference the other.
> 

Yes, this is probably one to be reflected globally. Also on your previous
comment about priority, I believe that a flow for ordering guarantees
should be a combination of queue_id, flow_id and priority. Two packets with
different priorities should expect to be reordered, since that tends to be
what priority implies.

> > + * of an event queue can be scheduled only to a
> >    * single port at a time. The port is guaranteed to have exclusive (atomic)
> >    * access to the associated flow context, which enables the user to avoid SW
> >    * synchronization. Atomic flows also help to maintain event ordering
> 
> "help" here needs to go, I think. It sounds like a best-effort affair. The
> atomic queue ordering guarantees (or the lack thereof) should be spelled
> out.
> 
> "Event order in an atomic flow is maintained."

Ack.

> 
> > - * since only one port at a time can process events from a flow of an
> > + * since only one port at a time can process events from each flow of an
> >    * event queue.
> 
> Yes, and *but also since* the event device is not reshuffling events
> enqueued to an atomic queue. And that's more complicated than just something
> that falls out of atomicity, especially if you assume that FORWARD type
> enqueues are not ordered with other FORWARD type enqueues on a different
> port.
> 

Ack.

> >    *
> > - * The atomic queue synchronization context is dedicated to the port until
> > + * The atomic queue synchronization context for a flow is dedicated to the port until
> 
> What is an "atomic queue synchronization context" (except for something that
> makes for long sentences).
> 

Yes, it's rather wordy. I like the idea of using lock terminology you
suggest. The use of the word "contexts" in relation to atomic/ordered I
find confusing myself too.

> How about:
> "The atomic flow is locked to the port until /../"
> 
> You could also used the word "bound" instead of "locked".
> 
> >    * application call rte_event_dequeue_burst() from the same port,
> >    * which implicitly releases the context. User may allow the scheduler to
> >    * release the context earlier than that by invoking rte_event_enqueue_burst()
> > - * with RTE_EVENT_OP_RELEASE operation.
> > + * with RTE_EVENT_OP_RELEASE operation for each event from that flow. The context
> > + * is only released once the last event from the flow, outstanding on the port,
> > + * is released. So long as there is one event from an atomic flow scheduled to
> > + * a port/core (including any events in the port's dequeue queue, not yet read
> > + * by the application), that port will hold the synchronization context.
> 
> In case you like the "atomic flow locked/bound to port", this part would
> also need updating.
> 
> Maybe here is a good place to add a note on memory ordering and event
> ordering.
> 
> "Any memory stores done as a part of event processing will be globally
> visible before the next event in the same atomic flow is dequeued on a
> different lcore."
> 
> I.e., enqueue includes write barrier before the event can be seen.
> 
> One should probably mentioned a rmb in dequeue as well.
> 

Do we think that that is necessary? I can add it, but I would have thought
that - as with rings - it could be assumed.

/Bruce
  
Bruce Richardson Jan. 31, 2024, 5:54 p.m. UTC | #3
On Tue, Jan 23, 2024 at 05:19:18PM +0100, Mattias Rönnblom wrote:
> On 2024-01-19 18:43, Bruce Richardson wrote:
> > The description of ordered and atomic scheduling given in the eventdev
> > doxygen documentation was not always clear. Try and simplify this so
> > that it is clearer for the end-user of the application
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> > 
> > NOTE TO REVIEWERS:
> > I've updated this based on my understanding of what these scheduling
> > types are meant to do. It matches my understanding of the support
> > offered by our Intel DLB2 driver, as well as the SW eventdev, and I
> > believe the DSW eventdev too. If it does not match the behaviour of
> > other eventdevs, let's have a discussion to see if we can reach a good
> > definition of the behaviour that is common.
> > ---
> >   lib/eventdev/rte_eventdev.h | 47 ++++++++++++++++++++-----------------
> >   1 file changed, 25 insertions(+), 22 deletions(-)
> > 
> > diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
> > index 2c6576e921..cb13602ffb 100644
> > --- a/lib/eventdev/rte_eventdev.h
> > +++ b/lib/eventdev/rte_eventdev.h
> > @@ -1313,26 +1313,24 @@ struct rte_event_vector {
> >   #define RTE_SCHED_TYPE_ORDERED          0
> >   /**< Ordered scheduling
> >    *
> > - * Events from an ordered flow of an event queue can be scheduled to multiple
> > + * Events from an ordered event queue can be scheduled to multiple
> 
> What is the rationale for this change?
> 
> An implementation that impose a total order on all events on a particular
> ordered queue will still adhere to the current, more relaxed, per-flow
> ordering semantics.
> 
> An application wanting a total order would just set the flow id to 0 on all
> events destined that queue, and it would work on all event devices.
> 
> Why don't you just put a note in the DLB driver saying "btw it's total
> order", so any application where per-flow ordering is crucial for
> performance (i.e., where the potentially needless head-of-line blocking is
> an issue) can use multiple queues when running with the DLB.
> 
> In the API as-written, the app is free to express more relaxed ordering
> requirements (i.e., to have multiple flows) and it's up to the event device
> to figure out if it's in a position where it can translate this to lower
> latency.
> 
> >    * ports for concurrent processing while maintaining the original event order.
> 
> Maybe it's worth mentioning what is the original event order. "(i.e., the
> order in which the events were enqueued to the queue)". Especially since one
> like to specify what ordering guarantees one have of events enqueued to the
> same queue on different ports and by different lcores).
> 
> I don't know where that information should go though, since it's relevant
> for both atomic and ordered-type queues.
> 
> >    * This scheme enables the user to achieve high single flow throughput by
> > - * avoiding SW synchronization for ordering between ports which bound to cores.
> > - *
> > - * The source flow ordering from an event queue is maintained when events are
> > - * enqueued to their destination queue within the same ordered flow context.
> > - * An event port holds the context until application call
> > - * rte_event_dequeue_burst() from the same port, which implicitly releases
> > - * the context.
> > - * User may allow the scheduler to release the context earlier than that
> > - * by invoking rte_event_enqueue_burst() with RTE_EVENT_OP_RELEASE operation.
> > - *
> > - * Events from the source queue appear in their original order when dequeued
> > - * from a destination queue.
> > - * Event ordering is based on the received event(s), but also other
> > - * (newly allocated or stored) events are ordered when enqueued within the same
> > - * ordered context. Events not enqueued (e.g. released or stored) within the
> > - * context are  considered missing from reordering and are skipped at this time
> > - * (but can be ordered again within another context).
> > + * avoiding SW synchronization for ordering between ports which are polled by
> > + * different cores.
> > + *
> > + * As events are scheduled to ports/cores, the original event order from the
> > + * source event queue is recorded internally in the scheduler. As events are
> > + * returned (via FORWARD type enqueue) to the scheduler, the original event
> > + * order is restored before the events are enqueued into their new destination
> > + * queue.
> 
> Delete the first sentence on implementation.
> 
> "As events are re-enqueued to the next queue (with the op field set to
> RTE_EVENT_OP_FORWARD), the event device restores the original event order
> before the events arrive on the destination queue."
> 

This whole section on ordered processing I'm reworking quite extensively
for v3, and hopefully I've taken all your comments into account. Finding it
really hard to try and explain it all simply and clearly. Please re-review
this part when I get the v3 finished and sent!

> > + *
> > + * Any events not forwarded, ie. dropped explicitly via RELEASE or implicitly
> > + * released by the next dequeue from a port, are skipped by the reordering
> > + * stage and do not affect the reordering of returned events.
> > + *
> > + * The ordering behaviour of NEW events with respect to FORWARD events is
> > + * undefined and implementation dependent.
> 
> For some reason I find this a little vague. "NEW and FORWARD events enqueued
> to a queue are not ordered in relation to each other (even if the flow id is
> the same)."
> 
> I think I agree that NEW shouldn't be ordered vis-a-vi FORWARD, but maybe
> one should say that an event device should avoid excessive reordering NEW
> and FORWARD events.
> 
> I think it would also be helpful to address port-to-port ordering guarantees
> (or a lack thereof).
> 
> "Events enqueued on one port are not ordered in relation to events enqueued
> on some other port."
> 
> Or are they? Not in DSW, at least, and I'm not sure I see a use case for
> such a guarantee, but it's a little counter-intuitive to have them
> potentially re-shuffled.
> 
> (This is also relevant for atomic queues.)
> 
> >    *
> >    * @see rte_event_queue_setup(), rte_event_dequeue_burst(), RTE_EVENT_OP_RELEASE
> >    */
> > @@ -1340,18 +1338,23 @@ struct rte_event_vector {
> >   #define RTE_SCHED_TYPE_ATOMIC           1
> >   /**< Atomic scheduling
> >    *
> > - * Events from an atomic flow of an event queue can be scheduled only to a
> > + * Events from an atomic flow, identified by @ref rte_event.flow_id,
> 
> A flow is identified by the combination of queue_id and flow_id, so if you
> reference one you should also reference the other.
> 

This is done in v3. I have mention of what defines in flow in both comments
for ordered and atomic.

> > + * of an event queue can be scheduled only to a
> >    * single port at a time. The port is guaranteed to have exclusive (atomic)
> >    * access to the associated flow context, which enables the user to avoid SW
> >    * synchronization. Atomic flows also help to maintain event ordering
> 
> "help" here needs to go, I think. It sounds like a best-effort affair. The
> atomic queue ordering guarantees (or the lack thereof) should be spelled
> out.
> 
> "Event order in an atomic flow is maintained."
> 
> > - * since only one port at a time can process events from a flow of an
> > + * since only one port at a time can process events from each flow of an
> >    * event queue.
> 
> Yes, and *but also since* the event device is not reshuffling events
> enqueued to an atomic queue. And that's more complicated than just something
> that falls out of atomicity, especially if you assume that FORWARD type
> enqueues are not ordered with other FORWARD type enqueues on a different
> port.
> 
> >    *
> > - * The atomic queue synchronization context is dedicated to the port until
> > + * The atomic queue synchronization context for a flow is dedicated to the port until
> 
> What is an "atomic queue synchronization context" (except for something that
> makes for long sentences).
> 
> How about:
> "The atomic flow is locked to the port until /../"
> 
> You could also used the word "bound" instead of "locked".
> 

Going with the term "lock" for v3.

> >    * application call rte_event_dequeue_burst() from the same port,
> >    * which implicitly releases the context. User may allow the scheduler to
> >    * release the context earlier than that by invoking rte_event_enqueue_burst()
> > - * with RTE_EVENT_OP_RELEASE operation.
> > + * with RTE_EVENT_OP_RELEASE operation for each event from that flow. The context
> > + * is only released once the last event from the flow, outstanding on the port,
> > + * is released. So long as there is one event from an atomic flow scheduled to
> > + * a port/core (including any events in the port's dequeue queue, not yet read
> > + * by the application), that port will hold the synchronization context.
> 
> In case you like the "atomic flow locked/bound to port", this part would
> also need updating.
> 
> Maybe here is a good place to add a note on memory ordering and event
> ordering.
> 
> "Any memory stores done as a part of event processing will be globally
> visible before the next event in the same atomic flow is dequeued on a
> different lcore."
> 
> I.e., enqueue includes write barrier before the event can be seen.
> 
> One should probably mentioned a rmb in dequeue as well.
> 
Not adding memory ordering in v3. If necessary we can add it later in
another patch.

/Bruce
  

Patch

diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
index 2c6576e921..cb13602ffb 100644
--- a/lib/eventdev/rte_eventdev.h
+++ b/lib/eventdev/rte_eventdev.h
@@ -1313,26 +1313,24 @@  struct rte_event_vector {
 #define RTE_SCHED_TYPE_ORDERED          0
 /**< Ordered scheduling
  *
- * Events from an ordered flow of an event queue can be scheduled to multiple
+ * Events from an ordered event queue can be scheduled to multiple
  * ports for concurrent processing while maintaining the original event order.
  * This scheme enables the user to achieve high single flow throughput by
- * avoiding SW synchronization for ordering between ports which bound to cores.
- *
- * The source flow ordering from an event queue is maintained when events are
- * enqueued to their destination queue within the same ordered flow context.
- * An event port holds the context until application call
- * rte_event_dequeue_burst() from the same port, which implicitly releases
- * the context.
- * User may allow the scheduler to release the context earlier than that
- * by invoking rte_event_enqueue_burst() with RTE_EVENT_OP_RELEASE operation.
- *
- * Events from the source queue appear in their original order when dequeued
- * from a destination queue.
- * Event ordering is based on the received event(s), but also other
- * (newly allocated or stored) events are ordered when enqueued within the same
- * ordered context. Events not enqueued (e.g. released or stored) within the
- * context are  considered missing from reordering and are skipped at this time
- * (but can be ordered again within another context).
+ * avoiding SW synchronization for ordering between ports which are polled by
+ * different cores.
+ *
+ * As events are scheduled to ports/cores, the original event order from the
+ * source event queue is recorded internally in the scheduler. As events are
+ * returned (via FORWARD type enqueue) to the scheduler, the original event
+ * order is restored before the events are enqueued into their new destination
+ * queue.
+ *
+ * Any events not forwarded, ie. dropped explicitly via RELEASE or implicitly
+ * released by the next dequeue from a port, are skipped by the reordering
+ * stage and do not affect the reordering of returned events.
+ *
+ * The ordering behaviour of NEW events with respect to FORWARD events is
+ * undefined and implementation dependent.
  *
  * @see rte_event_queue_setup(), rte_event_dequeue_burst(), RTE_EVENT_OP_RELEASE
  */
@@ -1340,18 +1338,23 @@  struct rte_event_vector {
 #define RTE_SCHED_TYPE_ATOMIC           1
 /**< Atomic scheduling
  *
- * Events from an atomic flow of an event queue can be scheduled only to a
+ * Events from an atomic flow, identified by @ref rte_event.flow_id,
+ * of an event queue can be scheduled only to a
  * single port at a time. The port is guaranteed to have exclusive (atomic)
  * access to the associated flow context, which enables the user to avoid SW
  * synchronization. Atomic flows also help to maintain event ordering
- * since only one port at a time can process events from a flow of an
+ * since only one port at a time can process events from each flow of an
  * event queue.
  *
- * The atomic queue synchronization context is dedicated to the port until
+ * The atomic queue synchronization context for a flow is dedicated to the port until
  * application call rte_event_dequeue_burst() from the same port,
  * which implicitly releases the context. User may allow the scheduler to
  * release the context earlier than that by invoking rte_event_enqueue_burst()
- * with RTE_EVENT_OP_RELEASE operation.
+ * with RTE_EVENT_OP_RELEASE operation for each event from that flow. The context
+ * is only released once the last event from the flow, outstanding on the port,
+ * is released. So long as there is one event from an atomic flow scheduled to
+ * a port/core (including any events in the port's dequeue queue, not yet read
+ * by the application), that port will hold the synchronization context.
  *
  * @see rte_event_queue_setup(), rte_event_dequeue_burst(), RTE_EVENT_OP_RELEASE
  */