From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 8CA8042E00; Sun, 9 Jul 2023 05:23:54 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1576040DF8; Sun, 9 Jul 2023 05:23:54 +0200 (CEST) Received: from szxga08-in.huawei.com (szxga08-in.huawei.com [45.249.212.255]) by mails.dpdk.org (Postfix) with ESMTP id A4BFA40A8A for ; Sun, 9 Jul 2023 05:23:51 +0200 (CEST) Received: from dggpeml100024.china.huawei.com (unknown [172.30.72.53]) by szxga08-in.huawei.com (SkyGuard) with ESMTP id 4QzCBv0bfsz1FDgk; Sun, 9 Jul 2023 11:23:19 +0800 (CST) Received: from [10.67.100.224] (10.67.100.224) by dggpeml100024.china.huawei.com (7.185.36.115) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.27; Sun, 9 Jul 2023 11:23:49 +0800 Subject: Re: [PATCH v4] dmadev: add tracepoints To: Thomas Monjalon CC: , Kevin Laatz , Bruce Richardson , , References: <20230412024808.41339-1-fengchengwen@huawei.com> <20230526084213.30586-1-fengchengwen@huawei.com> <3922895.oiGErgHkdL@thomas> From: fengchengwen Message-ID: <1f0b8df9-6a38-57e1-0d9e-ce031297f191@huawei.com> Date: Sun, 9 Jul 2023 11:23:48 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 MIME-Version: 1.0 In-Reply-To: <3922895.oiGErgHkdL@thomas> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.100.224] X-ClientProxiedBy: dggems703-chm.china.huawei.com (10.3.19.180) To dggpeml100024.china.huawei.com (7.185.36.115) X-CFilter-Loop: Reflected X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Hi Thomas, On 2023/7/7 18:40, Thomas Monjalon wrote: > 26/05/2023 10:42, Chengwen Feng: >> Add tracepoints at important APIs for tracing support. >> >> Signed-off-by: Chengwen Feng >> Acked-by: Morten Brørup >> >> --- >> v4: Fix asan smoke fail. >> v3: Address Morten's comment: >> Move stats_get and vchan_status and to trace_fp.h. >> v2: Address Morten's comment: >> Make stats_get as fast-path trace-points. >> Place fast-path trace-point functions behind in version.map. > > There are more things to fix. > First you must export rte_dmadev_trace_fp.h as it is included by rte_dmadev.h. It was already included by rte_dmadev.h: diff --git a/lib/dmadev/rte_dmadev.h b/lib/dmadev/rte_dmadev.h index e61d71959e..e792b90ef8 100644 --- a/lib/dmadev/rte_dmadev.h +++ b/lib/dmadev/rte_dmadev.h @@ -796,6 +796,7 @@ struct rte_dma_sge { }; #include "rte_dmadev_core.h" +#include "rte_dmadev_trace_fp.h" > Note: you could have caught this if testing the example app for DMA. > Second, you must avoid structs and enum in this header file, Let me explain the #if #endif logic: For the function: uint16_t rte_dma_completed(int16_t dev_id, uint16_t vchan, const uint16_t nb_cpls, uint16_t *last_idx, bool *has_error) The common trace implementation: RTE_TRACE_POINT_FP( rte_dma_trace_completed, RTE_TRACE_POINT_ARGS(int16_t dev_id, uint16_t vchan, const uint16_t nb_cpls, uint16_t *last_idx, bool *has_error, uint16_t ret), rte_trace_point_emit_i16(dev_id); rte_trace_point_emit_u16(vchan); rte_trace_point_emit_u16(nb_cpls); rte_trace_point_emit_ptr(idx_val); rte_trace_point_emit_ptr(has_error); rte_trace_point_emit_u16(ret); ) But it has a problem: for pointer parameter (e.g. last_idx and has_error), only record the pointer value (i.e. address value). I think the pointer value has no mean (in particular, many of there pointers are stack variables), the value of the pointer point to is meaningful. So I add the pointer reference like below (as V3 did): RTE_TRACE_POINT_FP( rte_dma_trace_completed, RTE_TRACE_POINT_ARGS(int16_t dev_id, uint16_t vchan, const uint16_t nb_cpls, uint16_t *last_idx, bool *has_error, uint16_t ret), int has_error_val = *has_error; // pointer reference int last_idx_val = *last_idx; // pointer reference rte_trace_point_emit_i16(dev_id); rte_trace_point_emit_u16(vchan); rte_trace_point_emit_u16(nb_cpls); rte_trace_point_emit_int(last_idx_val); // record the value of pointer rte_trace_point_emit_int(has_error_val); // record the value of pointer rte_trace_point_emit_u16(ret); ) Unfortunately, the above lead to asan failed. because in: RTE_TRACE_POINT_REGISTER(rte_dma_trace_completed, lib.dmadev.completed) it will invoke rte_dma_trace_completed() with the parameter is undefined. To solve this problem, consider the rte_dmadev_trace_points.c will include rte_trace_point_register.h, and the rte_trace_point_register.h will defined macro: _RTE_TRACE_POINT_REGISTER_H_. so we update trace points as (as V4 did): RTE_TRACE_POINT_FP( rte_dma_trace_completed, RTE_TRACE_POINT_ARGS(int16_t dev_id, uint16_t vchan, const uint16_t nb_cpls, uint16_t *last_idx, bool *has_error, uint16_t ret), #ifdef _RTE_TRACE_POINT_REGISTER_H_ uint16_t __last_idx = 0; bool __has_error = false; last_idx = &__last_idx; // make sure the pointer has meaningful value. has_error = &__has_error; // so that the next pointer reference will work well. #endif /* _RTE_TRACE_POINT_REGISTER_H_ */ int has_error_val = *has_error; int last_idx_val = *last_idx; rte_trace_point_emit_i16(dev_id); rte_trace_point_emit_u16(vchan); rte_trace_point_emit_u16(nb_cpls); rte_trace_point_emit_int(last_idx_val); rte_trace_point_emit_int(has_error_val); rte_trace_point_emit_u16(ret); ) > otherwise it cannot be included alone. > Look at what is done in other *_trace_fp.h files. > > Whether enable_trace_fp is true or false, the v4 work well. Below is that run examples with enable_trace_fp=true. ./dpdk-test --file-prefix=feng123 --trace=lib.dmadev.* -l 10-11 EAL: Detected CPU lcores: 96 EAL: Detected NUMA nodes: 4 EAL: Detected static linkage of DPDK EAL: Multi-process socket /var/run/dpdk/feng123/mp_socket EAL: Selected IOVA mode 'VA' EAL: VFIO support initialized TELEMETRY: No legacy callbacks, legacy socket not created APP: HPET is not enabled, using TSC as default timer RTE>>dmadev_autotest skeldma_probe(): Create dma_skeleton dmadev with lcore-id -1 ### Test dmadev infrastructure using skeleton driver test_dma_get_dev_id_by_name Passed test_dma_is_valid_dev Passed test_dma_count Passed test_dma_info_get Passed test_dma_configure Passed test_dma_vchan_setup Passed test_dma_start_stop Passed test_dma_stats Passed test_dma_dump Passed test_dma_completed Passed test_dma_completed_status Passed Total tests : 11 Passed : 11 Failed : 0 ### Test dmadev instance 0 [dma_skeleton] DMA Dev 0: Running copy Tests Ops submitted: 85120 Ops completed: 85120 Errors: 0 DMA Dev 0: Running stop-start Tests Ops submitted: 1 Ops completed: 1 Errors: 0 DMA Dev 0: Running burst capacity Tests Ops submitted: 65536 Ops completed: 65536 Errors: 0 DMA Dev 0: device does not report errors, skipping error handling tests DMA Dev 0: No device fill support, skipping fill tests Test OK RTE>> RTE>>quit skeldma_remove(): Remove dma_skeleton dmadev EAL: Trace dir: /root/dpdk-traces/feng123-2023-07-09-PM-06-57-08 [localhost fengchengwen]# babeltrace /root/dpdk-traces/feng123-2023-07-09-PM-06-57-08 | grep dma | head -1 [18:54:27.066265250] (+?.?????????) lib.dmadev.copy: { cpu_id = 0xA, name = "dpdk-test" }, { dev_id = 0, vchan = 0x0, src = 0x17F266480, dst = 0x17F292280, length = 0x400, flags = 0x0, ret = 63340 } [localhost fengchengwen]# babeltrace /root/dpdk-traces/feng123-2023-07-09-PM-06-57-08 | grep completed | head -1 [18:54:27.066315770] (+0.000001670) lib.dmadev.completed: { cpu_id = 0xA, name = "dpdk-test" }, { dev_id = 0, vchan = 0x0, nb_cpls = 0x40, last_idx_val = 63296, has_error_val = 0, ret = 0x40 } > > . > Thanks