> From: fengchengwen [mailto:fengcheng...@huawei.com] > Sent: Thursday, 13 April 2023 08.30 > > On 2023/4/12 19:00, Morten Brørup wrote: > >> From: Chengwen Feng [mailto:fengcheng...@huawei.com] > >> Sent: Wednesday, 12 April 2023 04.48 > >> > >> Add tracepoints at important APIs for tracing support. > >> > >> Signed-off-by: Chengwen Feng <fengcheng...@huawei.com> > >> --- > > > > ... > > >> +) > >> + > >> +RTE_TRACE_POINT( > >> + rte_dma_trace_stats_get, > > > > This should be a fast path trace point. > > For reference, ethdev considers rte_eth_stats_get() a fast path function. > > Emm, I think it should discuss more, and make it clear. > > The cryptodev and dmadev trace-points both make 'rte_xxx_trace_stats_get' as a > slow path function.
Then those stats tracepoints should be fixed, so they behave like ethdev. > And it mainly refer to the fast path API (means if a API is fast path then > make it as a fast-path trace-points). Dataplane functions must obviously be RTE_TRACE_POINT_FP. But some non-dataplane functions must also be RTE_TRACE_POINT_FP. > > But the ethdev trace-points treats 'calls in loop function(such as > rte_eth_trace_stats_get/rte_eth_trace_link_get/...)' > as fast path trace-points. Yes. Such "grey area" functions must use FP trace. Please refer to the Techboard decision to treat them as FP trace [1]. [1]: http://inbox.dpdk.org/dev/2325762.trqCLbgVIZ@thomas/ The techboard made this decision when trace was introduced to ethdev, and it seems that it was not caught in review when trace was added to cryptodev. PS: The trace point macro names are unfortunate... Since RTE_TRACE_POINT_FP is not only for Fast Path functions, they should have been named RTE_TRACE_POINT and RTE_TRACE_POINT_SLOW instead of RTE_TRACE_POINT_FP and RTE_TRACE_POINT. But it is too late to change now. > > > > >> + RTE_TRACE_POINT_ARGS(int16_t dev_id, uint16_t vchan, > >> + struct rte_dma_stats *stats, int ret), > >> + rte_trace_point_emit_i16(dev_id); > >> + rte_trace_point_emit_u16(vchan); > >> + rte_trace_point_emit_u64(stats->submitted); > >> + rte_trace_point_emit_u64(stats->completed); > >> + rte_trace_point_emit_u64(stats->errors); > >> + rte_trace_point_emit_int(ret); > >> +) > >> + > > ... > > >> diff --git a/lib/dmadev/version.map b/lib/dmadev/version.map > >> index 7031d6b335..4ee1b3f74a 100644 > >> --- a/lib/dmadev/version.map > >> +++ b/lib/dmadev/version.map > >> @@ -1,6 +1,16 @@ > >> EXPERIMENTAL { > >> global: > >> > >> + # added in 23.07 > >> + __rte_dma_trace_burst_capacity; > >> + __rte_dma_trace_completed; > >> + __rte_dma_trace_completed_status; > >> + __rte_dma_trace_copy; > >> + __rte_dma_trace_copy_sg; > >> + __rte_dma_trace_fill; > >> + __rte_dma_trace_submit; > >> + > > > > Intuitively, I would suppose that the 23.07 functions should be listed after > the 21.11 functions, not before. > > +1, will fix in v2 > > > > >> + # added in 21.11 > > > > Good catch. > > > >> rte_dma_close; > >> rte_dma_configure; > >> rte_dma_count_avail; > >> -- > >> 2.17.1 > > > > > > . > >
