From: "Mattias Rönnblom" <mattias.ronnblom@ericsson.com>
To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>,
Thomas Monjalon <thomas@monjalon.net>
Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>,
Ray Kinsella <mdr@ashroe.eu>, "dev@dpdk.org" <dev@dpdk.org>,
"timothy.mcdaniel@intel.com" <timothy.mcdaniel@intel.com>,
Hemant Agrawal <hemant.agrawal@nxp.com>,
"sachin.saxena@oss.nxp.com" <sachin.saxena@oss.nxp.com>,
"liangma@liangbit.com" <liangma@liangbit.com>,
"peter.mccarthy@intel.com" <peter.mccarthy@intel.com>,
"harry.van.haaren@intel.com" <harry.van.haaren@intel.com>,
"erik.g.carrillo@intel.com" <erik.g.carrillo@intel.com>,
"abhinandan.gujjar@intel.com" <abhinandan.gujjar@intel.com>,
"jay.jayatheerthan@intel.com" <jay.jayatheerthan@intel.com>,
"anatoly.burakov@intel.com" <anatoly.burakov@intel.com>
Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
Date: Sun, 17 Jul 2022 20:23:17 +0000 [thread overview]
Message-ID: <d4f0668e-bd99-0e63-4dcb-8dfececb9804@ericsson.com> (raw)
In-Reply-To: <PH0PR18MB4086A11EB5CC1FFE0D48F637DE8B9@PH0PR18MB4086.namprd18.prod.outlook.com>
On 2022-07-15 15:16, Pavan Nikhilesh Bhagavatula wrote:
>
>
>> -----Original Message-----
>> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>> Sent: Friday, July 15, 2022 1:26 PM
>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Thomas
>> Monjalon <thomas@monjalon.net>
>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
>> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com; Hemant
>> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
>> liangma@liangbit.com; peter.mccarthy@intel.com;
>> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
>> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
>> anatoly.burakov@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.ronnblom@ericsson.com>
>>>> Sent: Thursday, July 14, 2022 4:13 PM
>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Thomas
>>>> Monjalon <thomas@monjalon.net>
>>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
>>>> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com;
>> Hemant
>>>> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
>>>> liangma@liangbit.com; peter.mccarthy@intel.com;
>>>> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
>>>> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
>>>> anatoly.burakov@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.ronnblom@ericsson.com>
>>>>>> Sent: Wednesday, July 13, 2022 5:45 PM
>>>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>;
>> Thomas
>>>>>> Monjalon <thomas@monjalon.net>
>>>>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
>>>>>> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com;
>>>> Hemant
>>>>>> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
>>>>>> liangma@liangbit.com; peter.mccarthy@intel.com;
>>>>>> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
>>>>>> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
>>>>>> anatoly.burakov@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.ronnblom@ericsson.com>
>>>>>>>> Sent: Wednesday, July 13, 2022 2:39 PM
>>>>>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>;
>>>> Thomas
>>>>>>>> Monjalon <thomas@monjalon.net>
>>>>>>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
>>>>>>>> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com;
>>>>>> Hemant
>>>>>>>> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
>>>>>>>> liangma@liangbit.com; peter.mccarthy@intel.com;
>>>>>>>> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
>>>>>>>> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
>>>>>>>> anatoly.burakov@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.mcdaniel@intel.com;
>>>>>>>>> hemant.agrawal@nxp.com;
>>>>>>>>> sachin.saxena@oss.nxp.com;
>>>>>>>>> mattias.ronnblom@ericsson.com;
>>>>>>>>> jerinj@marvell.com;
>>>>>>>>> liangma@liangbit.com;
>>>>>>>>> peter.mccarthy@intel.com;
>>>>>>>>> harry.van.haaren@intel.com;
>>>>>>>>> erik.g.carrillo@intel.com;
>>>>>>>>> abhinandan.gujjar@intel.com;
>>>>>>>>> jay.jayatheerthan@intel.com;
>>>>>>>>> mdr@ashroe.eu;
>>>>>>>>> anatoly.burakov@intel.com;
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> -----Original Message-----
>>>>>>>>>> From: Thomas Monjalon <thomas@monjalon.net>
>>>>>>>>>> Sent: Tuesday, July 12, 2022 8:31 PM
>>>>>>>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
>>>>>>>>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
>>>>>>>>>> <mdr@ashroe.eu>; dev@dpdk.org; Pavan Nikhilesh Bhagavatula
>>>>>>>>>> <pbhagavatula@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, pbhagavatula@marvell.com:
>>>>>>>>>>> From: Pavan Nikhilesh <pbhagavatula@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 <pbhagavatula@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.
>>>>>>>
>>>>>
>>>
>
prev parent reply other threads:[~2022-07-17 20:23 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-27 9:57 pbhagavatula
2022-06-27 9:57 ` [PATCH 2/2] eventdev: add function to enq new events to the same queue pbhagavatula
2022-07-11 14:54 ` [PATCH 1/2] doc: add enqueue depth for new event type Jerin Jacob
2022-07-12 15:01 ` Thomas Monjalon
2022-07-12 18:11 ` [EXT] " Pavan Nikhilesh Bhagavatula
2022-07-12 20:47 ` Thomas Monjalon
2022-07-13 3:15 ` Hemant Agrawal
2022-07-13 9:08 ` Mattias Rönnblom
2022-07-13 10:40 ` Pavan Nikhilesh Bhagavatula
2022-07-13 12:15 ` Mattias Rönnblom
2022-07-14 6:32 ` Pavan Nikhilesh Bhagavatula
2022-07-14 9:45 ` Van Haaren, Harry
2022-07-14 10:53 ` Mattias Rönnblom
2022-07-14 14:44 ` Pavan Nikhilesh Bhagavatula
2022-07-15 7:43 ` Mattias Rönnblom
2022-07-14 10:43 ` Mattias Rönnblom
2022-07-14 16:42 ` Pavan Nikhilesh Bhagavatula
2022-07-14 16:53 ` Van Haaren, Harry
2022-07-14 16:57 ` Van Haaren, Harry
2022-07-15 8:13 ` Mattias Rönnblom
2022-07-17 12:38 ` Thomas Monjalon
2022-07-14 18:01 ` Pavan Nikhilesh Bhagavatula
2022-07-15 7:49 ` Van Haaren, Harry
2022-07-15 13:09 ` Pavan Nikhilesh Bhagavatula
2022-07-15 7:56 ` Mattias Rönnblom
2022-07-15 13:16 ` Pavan Nikhilesh Bhagavatula
2022-07-17 20:23 ` Mattias Rönnblom [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d4f0668e-bd99-0e63-4dcb-8dfececb9804@ericsson.com \
--to=mattias.ronnblom@ericsson.com \
--cc=abhinandan.gujjar@intel.com \
--cc=anatoly.burakov@intel.com \
--cc=dev@dpdk.org \
--cc=erik.g.carrillo@intel.com \
--cc=harry.van.haaren@intel.com \
--cc=hemant.agrawal@nxp.com \
--cc=jay.jayatheerthan@intel.com \
--cc=jerinj@marvell.com \
--cc=liangma@liangbit.com \
--cc=mdr@ashroe.eu \
--cc=pbhagavatula@marvell.com \
--cc=peter.mccarthy@intel.com \
--cc=sachin.saxena@oss.nxp.com \
--cc=thomas@monjalon.net \
--cc=timothy.mcdaniel@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).