Re: [PATCH 24.03 v2 2/9] eventdev: increase flexibility of all-types flag

[Bruce Richardson <bruce.richardson@intel.com>]

On Thu, Nov 23, 2023 at 09:37:58AM +0530, Jerin Jacob wrote:
> On Tue, Nov 21, 2023 at 5:25 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > Rather than requiring that any device advertising the
> > RTE_EVENT_DEV_CAP_QUEUE_ALL_TYPES flag support all of atomic, ordered
> > and parallel scheduling, we can redefine the field so that it basically
> > means that you don't need to specify the queue scheduling type at config
> > time. Instead all types of supported events can be sent to all queues.
> >
> > Suggested-by: Mattias Rönnblom <hofors@lysator.liu.se>
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >  lib/eventdev/rte_eventdev.h | 15 ++++++++++++---
> >  1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
> > index d48957362c..1c5043de26 100644
> > --- a/lib/eventdev/rte_eventdev.h
> > +++ b/lib/eventdev/rte_eventdev.h
> > @@ -250,11 +250,20 @@ struct rte_event;
> >   * @see rte_event_dequeue_burst()
> >   */
> >  #define RTE_EVENT_DEV_CAP_QUEUE_ALL_TYPES     (1ULL << 3)
> > -/**< Event device is capable of enqueuing events of any type to any queue.
> > +/**< Event device is capable of accepting enqueued events, of any type
> > + * advertised as supported by the device, to all destination queues.
> > + *
> > + * When this capability is set, the "schedule_type" field of the
> > + * rte_event_queue_conf structure is ignored when a queue is being configured.
> 
> can we also add something like below or so to above line
> 
> rte_event_queue_conf structure is ignored when a queue is being
> configured instead rte_event::sched_type
> shall be used.
> 
Preparing v3 now and including this doc change. However, I'm also wondering
about the correct behaviour when this flag is not set. When the flag is not
set, the events enqueued should match the event type of the queue, but do
we need to enforce this, or should we?

Couple of options:
1. Actually enforce this, and state that it is an error to enqueue events
   with another scheduling type.
2. Explicitly not enforce this, and just state instead that the sched_type
   of events will be ignored.

Personally, I'd tend very much towards #2. because:
* it's easier for the app, as they can ignore the sched_type field through
  the pipeline if they want, relying on each queues type to do the right
  thing. This could be especially useful if they have fallback mechanisms
  to e.g. configure a queue as atomic if reordered is not supported etc.
  The downside is that for portable applications the sched type
  should always be set anyway, but the app doesn't lose anything in this
  case with #2 over #1.

* It's easier and more performant for the drivers, since it's one less
  check that should be performed on enqueue. The driver can just blindly
  override the sched_type provided with the queue config version.

I actually think an extension of #2 would also be nice to have for
portability, whereby an app could explicitly configure a queue to only have
a scheduling type - event if it's all-types-capable - and thereafter never
have to set the sched_type field on events. The drivers would always
remember the queue-type and explicitly set that if necessary on enqueue

Thoughts?
/Bruce

PS: Going to send v3 now anyway, based on feedback thus far. If we get
quick consensus on above, I can roll it into a v4, otherwise we can look to
clarify that situation in a separate patch later.