> From: Mattias Rönnblom <[email protected]> > Sent: Wednesday, October 2, 2024 8:43 PM > To: Marchand, David <[email protected]> > Cc: Mattias Rönnblom <[email protected]>; [email protected] > <[email protected]>; Van Haaren, Harry <[email protected]>; Stefan > Sundkvist <[email protected]> > Subject: Re: [PATCH v2] service: extend service function call statistics > > On 2024-10-02 21:08, David Marchand wrote: > > On Wed, Oct 2, 2024 at 8:57 PM Mattias Rönnblom <[email protected]> > > wrote: > >>> Coverity flagged this patch with issue #445158. > >>> rte_delay_ms() is now unreachable. > >>> > >>> I suppose this delay is not that important for the unit test and we > >>> can remove it, but as I am not sure I'll let you have a look and send > >>> a fix. > >>> > >> > >> It works without it I think, but I would keep it, and add it to the > >> "case 0" branch. > >> > >> Let me know if you want a v2. > > > > Unfortunately, coverity is run only for merged stuff. > > So the report we got is for merged patch in the main repo. > > > > Please send a fix. > > > > Done. > > I decided I could just as well reintroduce the old behavior. You could > refactor away all this sleeping business I think, but that is for > another day.
Thanks for sending the patch Mattias - Coverity flagged me too, was intending on having a look with colleague Sean here today - instead we'll review your patch shortly. The delays were introduced so thread-spawns in the CI have some time to take affect, and to (intentionally) introduce some jitter so we cover all cases of one-thread-before-the-other checking poll counts and correct behaviour; this is useful as service-cores is inherently racy in terms of things running in parallel, hence the service_dummy() function has the delay. All this to say, lets leave the rte_delay_ms() in. Expect a response (hopefully Tested/Acked-by) soon! Regards -Harry
