> -----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. 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. > >>>>> > >>> > >
