On Fri, Feb 09, 2024 at 02:44:04PM +0530, Jerin Jacob wrote:
> On Fri, Feb 2, 2024 at 6:11 PM Bruce Richardson
> <bruce.richard...@intel.com> wrote:
> >
> > Clarify the meaning of the NEW, FORWARD and RELEASE event types.
> > For the fields in "rte_event" struct, enhance the comments on each to
> > clarify the field's use, and whether it is preserved between enqueue and
> > dequeue, and it's role, if any, in scheduling.
> >
> > Signed-off-by: Bruce Richardson <bruce.richard...@intel.com>
> > ---
> > V3: updates following review
> > ---
> > lib/eventdev/rte_eventdev.h | 161 +++++++++++++++++++++++++-----------
> > 1 file changed, 111 insertions(+), 50 deletions(-)
> >
> > diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
> > index 8d72765ae7..58219e027e 100644
> > --- a/lib/eventdev/rte_eventdev.h
> > +++ b/lib/eventdev/rte_eventdev.h
> > @@ -1463,47 +1463,54 @@ struct rte_event_vector {
> >
> > /* Event enqueue operations */
> > #define RTE_EVENT_OP_NEW 0
> > -/**< The event producers use this operation to inject a new event to the
> > - * event device.
> > +/**< The @ref rte_event.op field must be set to this operation type to
> > inject a new event,
> > + * i.e. one not previously dequeued, into the event device, to be scheduled
> > + * for processing.
> > */
> > #define RTE_EVENT_OP_FORWARD 1
> > -/**< The CPU use this operation to forward the event to different event
> > queue or
> > - * change to new application specific flow or schedule type to enable
> > - * pipelining.
> > +/**< The application must set the @ref rte_event.op field to this
> > operation type to return a
> > + * previously dequeued event to the event device to be scheduled for
> > further processing.
> > *
> > - * This operation must only be enqueued to the same port that the
> > + * This event *must* be enqueued to the same port that the
> > * event to be forwarded was dequeued from.
> > + *
> > + * The event's fields, including (but not limited to) flow_id, scheduling
> > type,
> > + * destination queue, and event payload e.g. mbuf pointer, may all be
> > updated as
> > + * desired by the application, but the @ref rte_event.impl_opaque field
> > must
> > + * be kept to the same value as was present when the event was dequeued.
> > */
> > #define RTE_EVENT_OP_RELEASE 2
> > /**< Release the flow context associated with the schedule type.
> > *
> > - * If current flow's scheduler type method is *RTE_SCHED_TYPE_ATOMIC*
> > - * then this function hints the scheduler that the user has completed
> > critical
> > - * section processing in the current atomic context.
> > - * The scheduler is now allowed to schedule events from the same flow from
> > - * an event queue to another port. However, the context may be still held
> > - * until the next rte_event_dequeue_burst() call, this call allows but
> > does not
> > - * force the scheduler to release the context early.
> > - *
> > - * Early atomic context release may increase parallelism and thus system
> > + * If current flow's scheduler type method is @ref RTE_SCHED_TYPE_ATOMIC
> > + * then this operation type hints the scheduler that the user has
> > completed critical
> > + * section processing for this event in the current atomic context, and
> > that the
> > + * scheduler may unlock any atomic locks held for this event.
> > + * If this is the last event from an atomic flow, i.e. all flow locks are
> > released,
>
>
> Similar comment as other email
> [Jerin] When there are multiple atomic events dequeue from @ref
> rte_event_dequeue_burst()
> for the same event queue, and it has same flow id then the lock is ....
>
> [Mattias]
> Yes, or maybe describing the whole lock/unlock state.
>
> "The conceptual per-queue-per-flow lock is in a locked state as long
> (and only as long) as one or more events pertaining to that flow were
> scheduled to the port in question, but are not yet released."
>
> Maybe it needs to be more meaty, describing what released means. I don't
> have the full context of the documentation in my head when I'm writing this.
>
Rather than trying to explain all again, I'm just going to put inline here a
cross-reference to the text on RTE_EVENT_TYPE_ATOMIC.
/Bruce
