> @Jerin, I'd like to know what do you think about my question/thoughts below
@Andrew Rybchenko see below. > > On 9/13/22 10:18, Andrew Rybchenko wrote: > > On 9/13/22 09:48, Ankur Dwivedi wrote: > >> Hi Andrew, > >> > >>> -----Original Message----- > >>> From: Andrew Rybchenko <[email protected]> > >>> Sent: Monday, September 12, 2022 4:30 PM > >>> To: Ankur Dwivedi <[email protected]>; [email protected] > >>> Cc: [email protected]; [email protected]; [email protected]; > >>> [email protected]; [email protected]; [email protected]; > >>> [email protected]; [email protected]; [email protected]; > >>> [email protected]; [email protected]; [email protected]; > >>> [email protected]; [email protected]; [email protected]; Igor > >>> Russkikh <[email protected]>; [email protected]; > >>> [email protected]; [email protected]; > >>> [email protected]; [email protected]; Jerin Jacob > >>> Kollanukkaran <[email protected]>; Maciej Czekaj [C] > >>> <[email protected]>; Shijith Thotton <[email protected]>; > >>> Srisivasubramanian Srinivasan <[email protected]>; Harman Kalra > >>> <[email protected]>; [email protected]; [email protected]; > >>> [email protected]; [email protected]; > >>> [email protected]; [email protected]; > >>> [email protected]; [email protected]; > >>> [email protected]; [email protected]; [email protected]; > >>> [email protected]; [email protected]; [email protected]; > >>> [email protected]; [email protected]; [email protected]; Nithin > >>> Kumar Dabilpuram <[email protected]>; Kiran Kumar Kokkilagadda > >>> <[email protected]>; Sunil Kumar Kori <[email protected]>; Satha > >>> Koteswara Rao Kottidi <[email protected]>; Liron Himi > >>> <[email protected]>; [email protected]; Radha Chintakuntla > >>> <[email protected]>; Veerasenareddy Burru <[email protected]>; > >>> Sathesh B Edara <[email protected]>; [email protected]; > >>> [email protected]; [email protected]; [email protected]; > >>> [email protected]; [email protected]; > >>> [email protected]; [email protected]; > >>> [email protected]; [email protected]; [email protected]; > >>> [email protected]; [email protected]; Rasesh Mody > >>> <[email protected]>; Shahed Shaikh <[email protected]>; Devendra > >>> Singh Rawat <[email protected]>; [email protected]; > >>> [email protected]; [email protected]; > >>> [email protected]; [email protected]; > >>> [email protected]; [email protected]; > >>> [email protected]; [email protected]; [email protected]; > >>> [email protected]; [email protected]; > >>> [email protected] > >>> Subject: [EXT] Re: [PATCH 1/6] ethdev: add trace points > >>> > >>> External Email > >>> > >>> ---------------------------------------------------------------------- > >>> On 8/4/22 16:44, Ankur Dwivedi wrote: > >>>> Add trace points for ethdev functions. > >>>> > >>>> Signed-off-by: Ankur Dwivedi <[email protected]> > >>>> --- > >>> > >>> [snip] > >>> > >>>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c index > >>>> 1979dc0850..a6fb370b22 100644 > >>>> --- a/lib/ethdev/rte_ethdev.c > >>>> +++ b/lib/ethdev/rte_ethdev.c > >>> > >>> [snip] > >>> > >>>> @@ -525,6 +536,7 @@ rte_eth_dev_owner_delete(const uint64_t > >>> owner_id) > >>>> > >>>> rte_spinlock_unlock(ð_dev_shared_data->ownership_lock); > >>>> > >>>> + rte_ethdev_trace_owner_delete(owner_id, ret); > >>> > >>> I'm wondering why trace is sometimes added in the middle of the > >>> function, > >>> but in the majority of cases it is added as the first or the last > >>> action. Is there > >>> any logical/guidelines behind it? > >> In this case for printing the return value the trace was added at the > >> end. I can change it if not required. > >> The logic which I used was to log at least the input arguments of a > >> function and in some cases also log important information(according to > >> me) if possible.For example in rte_eth_tx_buffer_count_callback() I > >> was also logging the count at the end. Similar logic in > >> rte_eth_link_get_nowait(). > >> Please let me know your views. > > > > The answer depends on purposes of tracing. I guess that the > > main goal is to understand what the application does. So, > > tracing without logging the result does not sound really > > useful. What's the point to see that application has tried > > to enable promiscuous mode without knowing the result if > > the attempt is successful or not? If failures are critical > > for the application functionality, hopefully it will > > result in error logging which could be used together with > > tracing to understand what happens. > > > > If so, it drives us to tracing nearby the end of the function > > when the function really has tried to do something. If there is > > no branching there we'll have some tracing of failures as well, > > but we definitely need to see the result in the trace point. Yes. It make sense to include the result. For example, rte_malloc etc we are already adding the result so the consumer of trace can get better view. > > > > I almost have no experience with tracing, so my thoughts > > could be wrong. > > Meanwhile I've updated the patch series as "Requested Changes" > since some fixes were promissed in v2.
