On 2022-07-15 15:16, Pavan Nikhilesh Bhagavatula wrote: > > >> -----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. >
OK. Can't this be communicate to the application by setting the max dequeue depth to 1? > 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. > Yeah, but why? How is this information useful? The only scenario I can think of is an application employing a fd.io VPP style SIMD-heavy "vectorized" design, with the per-burst processing progressing as a series of loops, one per layer (or "node"). If the event device can only hand you a single event at a time, then such an application would suffer, performance wise. In such a case, what is relevant is the maximum *dequeue* depth. For one-event-at-a-time type applications, it will just be a case of an unnecessary loop, which cost will be trivial. >> >> 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: > https://protect2.fireeye.com/v1/url?k=31323334-501d5122-313273af-454445555731-afc8d38dd7668c15&q=1&e=8100a5ca-5a51-46b2-91e1-1b6a49519b24&u=http%3A%2F%2Fgit.dpdk.org%2Fdpdk%2Ftree%2Fapp%2Ftest-eventdev%2Ftest_perf_common.c%23n545 > https://protect2.fireeye.com/v1/url?k=31323334-501d5122-313273af-454445555731-bb3f76042845f72f&q=1&e=8100a5ca-5a51-46b2-91e1-1b6a49519b24&u=http%3A%2F%2Fgit.dpdk.org%2Fdpdk%2Ftree%2Fapp%2Ftest-eventdev%2Fevt_common.h%23n99 > >> >>> 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. >>>>>>> >>>>> >>> >
