> -----Original Message----- > From: Mattias Rönnblom <[email protected]> > Sent: Friday, July 15, 2022 1:26 PM > To: Pavan Nikhilesh Bhagavatula <[email protected]>; Thomas > Monjalon <[email protected]> > Cc: Jerin Jacob Kollanukkaran <[email protected]>; Ray Kinsella > <[email protected]>; [email protected]; [email protected]; Hemant > Agrawal <[email protected]>; [email protected]; > [email protected]; [email protected]; > [email protected]; [email protected]; > [email protected]; [email protected]; > [email protected] > Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event > type > > On 2022-07-14 18:42, Pavan Nikhilesh Bhagavatula wrote: > > > > > >> -----Original Message----- > >> From: Mattias Rönnblom <[email protected]> > >> Sent: Thursday, July 14, 2022 4:13 PM > >> To: Pavan Nikhilesh Bhagavatula <[email protected]>; Thomas > >> Monjalon <[email protected]> > >> Cc: Jerin Jacob Kollanukkaran <[email protected]>; Ray Kinsella > >> <[email protected]>; [email protected]; [email protected]; > Hemant > >> Agrawal <[email protected]>; [email protected]; > >> [email protected]; [email protected]; > >> [email protected]; [email protected]; > >> [email protected]; [email protected]; > >> [email protected] > >> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new > event > >> type > >> > >> On 2022-07-14 08:32, Pavan Nikhilesh Bhagavatula wrote: > >>> > >>> > >>>> -----Original Message----- > >>>> From: Mattias Rönnblom <[email protected]> > >>>> Sent: Wednesday, July 13, 2022 5:45 PM > >>>> To: Pavan Nikhilesh Bhagavatula <[email protected]>; > Thomas > >>>> Monjalon <[email protected]> > >>>> Cc: Jerin Jacob Kollanukkaran <[email protected]>; Ray Kinsella > >>>> <[email protected]>; [email protected]; [email protected]; > >> Hemant > >>>> Agrawal <[email protected]>; [email protected]; > >>>> [email protected]; [email protected]; > >>>> [email protected]; [email protected]; > >>>> [email protected]; [email protected]; > >>>> [email protected] > >>>> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new > >> event > >>>> type > >>>> > >>>> On 2022-07-13 12:40, Pavan Nikhilesh Bhagavatula wrote: > >>>>> > >>>>> > >>>>>> -----Original Message----- > >>>>>> From: Mattias Rönnblom <[email protected]> > >>>>>> Sent: Wednesday, July 13, 2022 2:39 PM > >>>>>> To: Pavan Nikhilesh Bhagavatula <[email protected]>; > >> Thomas > >>>>>> Monjalon <[email protected]> > >>>>>> Cc: Jerin Jacob Kollanukkaran <[email protected]>; Ray Kinsella > >>>>>> <[email protected]>; [email protected]; [email protected]; > >>>> Hemant > >>>>>> Agrawal <[email protected]>; [email protected]; > >>>>>> [email protected]; [email protected]; > >>>>>> [email protected]; [email protected]; > >>>>>> [email protected]; [email protected]; > >>>>>> [email protected] > >>>>>> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new > >>>> event > >>>>>> type > >>>>>> > >>>>>> On 2022-07-12 20:11, Pavan Nikhilesh Bhagavatula wrote: > >>>>>>> +Cc > >>>>>>> [email protected]; > >>>>>>> [email protected]; > >>>>>>> [email protected]; > >>>>>>> [email protected]; > >>>>>>> [email protected]; > >>>>>>> [email protected]; > >>>>>>> [email protected]; > >>>>>>> [email protected]; > >>>>>>> [email protected]; > >>>>>>> [email protected]; > >>>>>>> [email protected]; > >>>>>>> [email protected]; > >>>>>>> [email protected]; > >>>>>>> > >>>>>>> > >>>>>>>> -----Original Message----- > >>>>>>>> From: Thomas Monjalon <[email protected]> > >>>>>>>> Sent: Tuesday, July 12, 2022 8:31 PM > >>>>>>>> To: Pavan Nikhilesh Bhagavatula <[email protected]> > >>>>>>>> Cc: Jerin Jacob Kollanukkaran <[email protected]>; Ray Kinsella > >>>>>>>> <[email protected]>; [email protected]; Pavan Nikhilesh Bhagavatula > >>>>>>>> <[email protected]> > >>>>>>>> Subject: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new > >> event > >>>>>> type > >>>>>>>> > >>>>>>>> External Email > >>>>>>>> > >>>>>>>> ---------------------------------------------------------------------- > >>>>>>>> I'm not your teacher, but please consider Cc'ing people outside of > >> your > >>>>>>>> company. > >>>>>>>> > >>>>>>> > >>>>>>> I generally add --to-cmd=./devtools/get-maintainer.sh but looks > like > >> it's > >>>>>> useless for > >>>>>>> sending deprecation notices. > >>>>>>> > >>>>>>> Maybe we should update it to include lib/driver maintainers when > diff > >>>> sees > >>>>>> deprecation.rst > >>>>>>> > >>>>>>>> > >>>>>>>> 27/06/2022 11:57, [email protected]: > >>>>>>>>> From: Pavan Nikhilesh <[email protected]> > >>>>>>>>> > >>>>>>>>> A new field ``max_event_port_enqueue_new_burst`` will be > >> added > >>>> to > >>>>>> the > >>>>>>>>> structure ``rte_event_dev_info``. The field defines the max > >> enqueue > >>>>>>>>> burst size of new events (OP_NEW) supported by the underlying > >>>> event > >>>>>>>>> device. > >>>>>>>>> > >>>>>>>>> Signed-off-by: Pavan Nikhilesh <[email protected]> > >>>>>>>>> --- > >>>>>>>>> doc/guides/rel_notes/deprecation.rst | 5 +++++ > >>>>>>>>> 1 file changed, 5 insertions(+) > >>>>>>>>> > >>>>>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst > >>>>>>>> b/doc/guides/rel_notes/deprecation.rst > >>>>>>>>> index 4e5b23c53d..071317e8e3 100644 > >>>>>>>>> --- a/doc/guides/rel_notes/deprecation.rst > >>>>>>>>> +++ b/doc/guides/rel_notes/deprecation.rst > >>>>>>>>> @@ -125,3 +125,8 @@ Deprecation Notices > >>>>>>>>> applications should be updated to use the ``dmadev`` library > >>>> instead, > >>>>>>>>> with the underlying HW-functionality being provided by the > >> ``ioat`` > >>>> or > >>>>>>>>> ``idxd`` dma drivers > >>>>>>>>> + > >>>>>>>>> +* eventdev: The structure ``rte_event_dev_info`` will be > >> extended > >>>> to > >>>>>>>> include the > >>>>>>>>> + max enqueue burst size of new events supported by the > >>>> underlying > >>>>>>>> event device. > >>>>>>>>> + A new field ``max_event_port_enqueue_new_burst`` will be > >>>> added > >>>>>> to > >>>>>>>> the structure > >>>>>>>>> + ``rte_event_dev_info`` in DPDK 22.11. > >>>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>> > >>>>>> Why is this needed, or useful? Why isn't called something with > >>>>>> "enqueue_depth" in it, like the already-present field? > >>>>>> > >>>>> > >>>>> This is for a case where enqueues with OP_FORWARD/OP_RELEASE > only > >>>> supports > >>>>> enqueue depth of 1. > >>>> > >>>> I assume it's for other cases as well? Any case when the event device > >>>> has a max forward enqueue depth != max new enqueue depth? > >>>> > >>> > >>> Yes, generally new events have much more flexibility than forwards > event. > >>> > >>>> I don't see why an event device would have such low max limit on the > >>>> number events enqueued. > >>> > >>> It depends on the number of scheduling contexts a given event port can > >> track. > >> > >> I have no idea what a "scheduling context" is (it's not something > >> related to the Eventdev APIs), but if you have a shortage of them (for > >> the moment, or always), the driver would just return a short count from > >> the enqueue burst function. What use has the application of knowing that > >> the event device can accept at most a certain number of events? > >> > >>> Anyway this would align to the current existing feature definitions. The > >> existing > >>> API only defines the enqueue size of fwd and release events i.e. > >> scheduling contexts > >>> already tracked by event device. > >> > >> The documentation of max_event_port_enqueue_depth says: > >> "Maximum number of events can be enqueued at a time from an event > port > >> by this device." > >> > >> From what I can tell, it says nothing about new versus forward, or > >> release type events. > >> > >>> NEW is always a special case as we are adding new scheduling contexts > to > >> the event > >>> device, the idea of this patch is to specify that NEW events wouldn’t > have > >> the same > >>> restrictions of fwd/release events. > >>> > >>> This would also allow us to craft optimized APIs such as > >>> https://urldefense.proofpoint.com/v2/url?u=https- > >> 3A__protect2.fireeye.com_v1_url-3Fk-3D31323334-2D501d5122- > 2D313273af- > >> 2D454445555731-2Def12ffaf4bdb568f-26q-3D1-26e-3Df0a92ad3-2D1167- > >> 2D448d-2Dbe14-2D0987e939f019-26u-3Dhttps-253A-252F- > >> 252Fpatches.dpdk.org-252Fproject-252Fdpdk-252Fpatch- > >> 252F20220627095702.8047-2D2-2Dpbhagavatula-2540marvell.com- > >> 252F&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=E3SgYMjtKCMVsB- > >> fmvgGV3o-g_fjLhk5Pupi9ijohpc&m=iazRQtbylIuGXRfj2NPpf_wwG- > >> > ppSwFOPKclj5zcwu2pQG9b7ovePiSBn9k4PjQZ&s=sTrEcWbst59oK_jYe6jXhya > >> CWwELib6Yr-3d9BccQt4&e= > >>> > >>> > >>>> If the underlying hardware has some limitations, > >>>> why not let the driver loop until back pressure occurs? Then you can > >> > >> You didn't answer this question. Why not let the driver loop, until you > >> for some reason or the other can't accept more events? > > > > CNXK event driver cannot accept forwarding(enq) more than one event > that has > > been dequeued. Enqueueing more than one event for > forwarding/releasing > > is a violation from HW perspective, this is currently announced by BURST > capability. > > But It can enqueue a burst if new events. > > > > OK, so the limitation we are discussing here is not really related to > enqueue bursts, but the number of allowed events that can be in-flight > (in-progress) on the eventdev port?
Yes that’s correct. In CNXK each event port tracks only one scheduling context (held on dequeue), and OP_FWD/OP_RELEASE translate to commands to the device to operate on the the scheduling context. There can be only one pending command per a "scheduling context" until the next dequeue. > > So what happens if you announce some larger max enqueue depth in > dev_info? If the application can't dequeue more than one event at a > time, which means it can't possible enqueue more than one forward event > at a time. Or am I missing something? Even with implicit release turned > off, the PMD can deny the application having more than one outstanding > event. My intention was to cleanly differentiate between OP_FWD and OP_NEW as it might mislead an application writer as he might interpret the max_enqueue_depth to be applicable for OP_FWD/OP_NEW unless explicitly stated. > > One issue is head-of-line blocking if you mix new and forward events, > but that you can solve on the application level by separating new and > forward bursts. That problem already exists, but for back pressure > scenarios, where new events are disallowed, but forward are not. > > > If you see the current example implementation we pick the worker based > on > > BURST capability for optimizing the enqueue/dequeue by providing a hint > > to the driver layer. > > > > What example? What does "pick the worker" mean? Pick worker functions: http://git.dpdk.org/dpdk/tree/app/test-eventdev/test_perf_common.c#n545 http://git.dpdk.org/dpdk/tree/app/test-eventdev/evt_common.h#n99 > > > Although, we could live with aggregating the events at driver layer based > on > > queue. We would still require announce burst capability for new events i.e. > > changes to the info structure. > > > >> > >>>> amortize the function call overhead and potentially other software > >>>> operations (credit system management etc) over multiple events. Plus, > >>>> the application doesn't need a special case for new events versus > >>>> forward type events, or this-versus-that event device. > >>>> > >>>>> Where as OP_NEW supports higher burst size. > >>>>> > >>>>> This is already tied into capability description : > >>>>> > >>>>> #define RTE_EVENT_DEV_CAP_BURST_MODE (1ULL << 4) > >>>>> /**< Event device is capable of operating in burst mode for > >>>> enqueue(forward, > >>>>> * release) and dequeue operation. If this capability is not set, > >> application > >>>>> * still uses the rte_event_dequeue_burst() and > >>>> rte_event_enqueue_burst() but > >>>>> * PMD accepts only one event at a time. > >>>>> * > >>>>> * @see rte_event_dequeue_burst() rte_event_enqueue_burst() > >>>>> */ > >>>>> > >>>>>> I think I would rather remove all fields related to the max > >>>>>> enqueue/dequeue burst sizes. They serve no purpose, as far as I > see. > >> If > >>>>>> you have some HW limit on the maximum number of new events it > can > >>>>>> accept, have the driver retry until backpressure occurs. > >>>>> > >>> > >
