On 2022-07-15 15:16, Pavan Nikhilesh Bhagavatula wrote: > > >> -----Original Message----- >> From: Mattias Rönnblom <mattias.ronnb...@ericsson.com> >> Sent: Friday, July 15, 2022 1:26 PM >> To: Pavan Nikhilesh Bhagavatula <pbhagavat...@marvell.com>; Thomas >> Monjalon <tho...@monjalon.net> >> Cc: Jerin Jacob Kollanukkaran <jer...@marvell.com>; Ray Kinsella >> <m...@ashroe.eu>; dev@dpdk.org; timothy.mcdan...@intel.com; Hemant >> Agrawal <hemant.agra...@nxp.com>; sachin.sax...@oss.nxp.com; >> lian...@liangbit.com; peter.mccar...@intel.com; >> harry.van.haa...@intel.com; erik.g.carri...@intel.com; >> abhinandan.guj...@intel.com; jay.jayatheert...@intel.com; >> anatoly.bura...@intel.com >> 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 <mattias.ronnb...@ericsson.com> >>>> Sent: Thursday, July 14, 2022 4:13 PM >>>> To: Pavan Nikhilesh Bhagavatula <pbhagavat...@marvell.com>; Thomas >>>> Monjalon <tho...@monjalon.net> >>>> Cc: Jerin Jacob Kollanukkaran <jer...@marvell.com>; Ray Kinsella >>>> <m...@ashroe.eu>; dev@dpdk.org; timothy.mcdan...@intel.com; >> Hemant >>>> Agrawal <hemant.agra...@nxp.com>; sachin.sax...@oss.nxp.com; >>>> lian...@liangbit.com; peter.mccar...@intel.com; >>>> harry.van.haa...@intel.com; erik.g.carri...@intel.com; >>>> abhinandan.guj...@intel.com; jay.jayatheert...@intel.com; >>>> anatoly.bura...@intel.com >>>> 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 <mattias.ronnb...@ericsson.com> >>>>>> Sent: Wednesday, July 13, 2022 5:45 PM >>>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavat...@marvell.com>; >> Thomas >>>>>> Monjalon <tho...@monjalon.net> >>>>>> Cc: Jerin Jacob Kollanukkaran <jer...@marvell.com>; Ray Kinsella >>>>>> <m...@ashroe.eu>; dev@dpdk.org; timothy.mcdan...@intel.com; >>>> Hemant >>>>>> Agrawal <hemant.agra...@nxp.com>; sachin.sax...@oss.nxp.com; >>>>>> lian...@liangbit.com; peter.mccar...@intel.com; >>>>>> harry.van.haa...@intel.com; erik.g.carri...@intel.com; >>>>>> abhinandan.guj...@intel.com; jay.jayatheert...@intel.com; >>>>>> anatoly.bura...@intel.com >>>>>> 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 <mattias.ronnb...@ericsson.com> >>>>>>>> Sent: Wednesday, July 13, 2022 2:39 PM >>>>>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavat...@marvell.com>; >>>> Thomas >>>>>>>> Monjalon <tho...@monjalon.net> >>>>>>>> Cc: Jerin Jacob Kollanukkaran <jer...@marvell.com>; Ray Kinsella >>>>>>>> <m...@ashroe.eu>; dev@dpdk.org; timothy.mcdan...@intel.com; >>>>>> Hemant >>>>>>>> Agrawal <hemant.agra...@nxp.com>; sachin.sax...@oss.nxp.com; >>>>>>>> lian...@liangbit.com; peter.mccar...@intel.com; >>>>>>>> harry.van.haa...@intel.com; erik.g.carri...@intel.com; >>>>>>>> abhinandan.guj...@intel.com; jay.jayatheert...@intel.com; >>>>>>>> anatoly.bura...@intel.com >>>>>>>> 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 >>>>>>>>> timothy.mcdan...@intel.com; >>>>>>>>> hemant.agra...@nxp.com; >>>>>>>>> sachin.sax...@oss.nxp.com; >>>>>>>>> mattias.ronnb...@ericsson.com; >>>>>>>>> jer...@marvell.com; >>>>>>>>> lian...@liangbit.com; >>>>>>>>> peter.mccar...@intel.com; >>>>>>>>> harry.van.haa...@intel.com; >>>>>>>>> erik.g.carri...@intel.com; >>>>>>>>> abhinandan.guj...@intel.com; >>>>>>>>> jay.jayatheert...@intel.com; >>>>>>>>> m...@ashroe.eu; >>>>>>>>> anatoly.bura...@intel.com; >>>>>>>>> >>>>>>>>> >>>>>>>>>> -----Original Message----- >>>>>>>>>> From: Thomas Monjalon <tho...@monjalon.net> >>>>>>>>>> Sent: Tuesday, July 12, 2022 8:31 PM >>>>>>>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavat...@marvell.com> >>>>>>>>>> Cc: Jerin Jacob Kollanukkaran <jer...@marvell.com>; Ray Kinsella >>>>>>>>>> <m...@ashroe.eu>; dev@dpdk.org; Pavan Nikhilesh Bhagavatula >>>>>>>>>> <pbhagavat...@marvell.com> >>>>>>>>>> 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, pbhagavat...@marvell.com: >>>>>>>>>>> From: Pavan Nikhilesh <pbhagavat...@marvell.com> >>>>>>>>>>> >>>>>>>>>>> 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 <pbhagavat...@marvell.com> >>>>>>>>>>> --- >>>>>>>>>>> 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. >>>>>>> >>>>> >>> >
