On Sun, Oct 31, 2021 at 6:47 PM Jerin Jacob <jerinjac...@gmail.com> wrote: > > On Sat, Oct 30, 2021 at 10:49 PM Mattias Rönnblom > <mattias.ronnb...@ericsson.com> wrote: > > > > On 2021-10-29 17:17, Jerin Jacob wrote: > > > On Fri, Oct 29, 2021 at 8:33 PM Mattias Rönnblom > > > <mattias.ronnb...@ericsson.com> wrote: > > >> On 2021-10-29 16:38, Jerin Jacob wrote: > > >>> On Tue, Oct 26, 2021 at 11:02 PM Mattias Rönnblom > > >>> <mattias.ronnb...@ericsson.com> wrote: > > >>>> Extend Eventdev API to allow for event devices which require various > > >>>> forms of internal processing to happen, even when events are not > > >>>> enqueued to or dequeued from a port. > > >>>> > > >>>> PATCH v1: > > >>>> - Adapt to the move of fastpath function pointers out of > > >>>> rte_eventdev struct > > >>>> - Attempt to clarify how often the application is expected to > > >>>> call rte_event_maintain() > > >>>> - Add trace point > > >>>> RFC v2: > > >>>> - Change rte_event_maintain() return type to be consistent > > >>>> with the documentation. > > >>>> - Remove unused typedef from eventdev_pmd.h. > > >>>> > > >>>> Signed-off-by: Mattias Rönnblom <mattias.ronnb...@ericsson.com> > > >>>> Tested-by: Richard Eklycke <richard.ekly...@ericsson.com> > > >>>> Tested-by: Liron Himi <lir...@marvell.com> > > >>>> --- > > >>>> > > >>>> +/** > > >>>> + * Maintain an event device. > > >>>> + * > > >>>> + * This function is only relevant for event devices which has the > > >>>> + * RTE_EVENT_DEV_CAP_REQUIRES_MAINT flag set. Such devices require the > > >>>> + * application to call rte_event_maintain() on a port during periods > > >>>> + * which it is neither enqueuing nor dequeuing events from that > > >>>> + * port. > > >>> # We need to add "by the same core". Right? As other core such as > > >>> service core can not call rte_event_maintain() > > >> > > >> Do you mean by the same lcore thread that "owns" (dequeues and enqueues > > >> to) the port? Yes. I thought that was implicit, since eventdev port are > > >> not MT safe. I'll try to figure out some wording that makes that more > > >> clear. > > > OK. > > > > > >> > > >>> # Also, Incase of Adapters enqueue() happens, right? If so, either > > >>> above text is not correct. > > >>> # @Erik Gabriel Carrillo @Jayatheerthan, Jay @Gujjar, Abhinandan S > > >>> Please review 3/3 patch on adapter change. > > >>> Let me know you folks are OK with change or not or need more time to > > >>> analyze. > > >>> > > >>> If it need only for the adapter subsystem then can we make it an > > >>> internal API between DSW and adapters? > > >> > > >> No, it's needed for any producer-only eventdev ports, including any such > > >> ports used by the application. > > > > > > In that case, the code path in testeventdev, eventdev_pipeline, etc needs > > > to be updated. I am worried about the performance impact for the drivers > > > they > > > don't have such limitations. > > > > > > Applications that are using some other event device today, and don't > > care about DSW or potential future event devices > > requiringRTE_EVENT_DEV_CAP_REQUIRES_MAINT, won't be affected at all, > > except the ops struct will be 8 bytes larger. > > Ack. That's not an issue. > > > > > > > A rte_event_maintain() call on a device which doesn't need maintenance > > is just an inlined NULL compare on the ops struct field, which is > > frequently used and should be in a cache close to the core. In my > > benchmarks, I've been unable to measure any additional cost at all. > > > > > > I reviewed the test and example applications last time I attempted to > > upstream this patch set, and from what I remember there was nothing to > > update. Things might have changed and I might misremember, so I'll have > > a look again. > > > OK. > > > > > > > What's important to keep in mind is that applications (DPDK tests, > > examples, user applications etc.) that have producer-only ports or > > otherwise potentially leave eventdev ports "unattended" don't work with > > DSW today, unless the take the measures described in the DSW > > documentation (which for example the eventdev adapters do not). So > > rte_event_maintain() will not break anything that's not already broken. > > > OK. > > > > > > > > Why not have an additional config option in port_config which says > > > it is a producer-only port by an application and takes care of the driver. > > > > > > In the current adapters code, you are calling maintain() when enqueue > > > returns zero. > > > > > > rte_event_maintain() is called when no interaction with the event device > > has been done, during that service function call. That's the overall > > intention. > > > > In the RX adapter, zero flushed events can also mean that the RX adapter > > had buffered events it wanted to flush, but the event device didn't > > accept new events (i.e, back pressure). In that case, the > > rte_event_maintain() call is redundant, but harmless (both because it's > > very low overhead on DSW, and near-zero overhead on any other current > > event device). Plus, if you are back-pressured by the pipeline, RX is > > not the bottleneck so a tiny bit of extra overhead is not an issue. > > OK. > > Looks good to me. Since DSW has better performance than other SW > drivers, I think, it is OK > to take some limitations to the application. > > Please send the next version with the suggested documentation change. > > If there is no objection, I will merge it on Wednesday. (3rd Nov)
Applied v3 version to dpdk-next-net-eventdev/for-main. Thanks
