Re: [PATCH] dmadev: add tracepoints

[fengchengwen <fengchengwen@huawei.com>]

On 2023/4/12 19:00, Morten Brørup wrote:
>> From: Chengwen Feng [mailto:fengchengwen@huawei.com]
>> Sent: Wednesday, 12 April 2023 04.48
>>
>> Add tracepoints at important APIs for tracing support.
>>
>> Signed-off-by: Chengwen Feng <fengchengwen@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.
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).

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.

> 
>> +	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
> 
> 
> .
>