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 3BC6342E38; Mon, 10 Jul 2023 09:50:11 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 09A5140C35; Mon, 10 Jul 2023 09:50:11 +0200 (CEST) Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by mails.dpdk.org (Postfix) with ESMTP id EAF1240698 for ; Mon, 10 Jul 2023 09:50:08 +0200 (CEST) Received: from dggpeml100024.china.huawei.com (unknown [172.30.72.57]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4Qzx3c184BzqVWB; Mon, 10 Jul 2023 15:49:32 +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; Mon, 10 Jul 2023 15:50:04 +0800 Subject: Re: [PATCH v4] dmadev: add tracepoints To: Thomas Monjalon CC: , Kevin Laatz , Bruce Richardson , , References: <20230412024808.41339-1-fengchengwen@huawei.com> <3922895.oiGErgHkdL@thomas> <1f0b8df9-6a38-57e1-0d9e-ce031297f191@huawei.com> <7265847.4vTCxPXJkl@thomas> From: fengchengwen Message-ID: Date: Mon, 10 Jul 2023 15:50:04 +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: <7265847.4vTCxPXJkl@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: dggems705-chm.china.huawei.com (10.3.19.182) 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/10 14:49, Thomas Monjalon wrote: > 09/07/2023 05:23, fengchengwen: >> 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 > > This is the test application, not the example. > Please make sure examples/dma/ is compiling. Work well with examples/dma (compiled with enable_trace_fp=true). dpdk-dma -a 0000:7b:00.0 -a 0000:7d:00.0 --file-prefix=feng -l 10-11 --trace=lib.dmadev.* -- -c hw ./dpdk-dma, Worker Threads = 1, Copy Mode = hw, Updating MAC = enabled, Rx Queues = 1, Ring Size = 2048 Force Min Copy Size = 0 Packet Data Room Size = 2048 Statistics for port 0 ------------------------------ Packets sent: 26588760 Packets received: 26589288 Packets dropped on tx: 528 Packets dropped on copy: 0 DMA channel 0 Total submitted ops: 26589288 Total completed ops: 26589288 Total failed ops: 0 Aggregate statistics =============================== Total packets Tx: 0 [pkt/s] Total packets Rx: 0 [pkt/s] Total packets dropped: 0 [pkt/s] Total submitted ops: 0 [ops/s] Total completed ops: 0 [ops/s] Total failed ops: 0 [ops/s] ==================================================== Closing port 0 0000:7d:00.0 hns3_dev_close(): Close port 0 finished Stopping dmadev 0 EAL: Trace dir: /root/dpdk-traces/feng-2023-07-10-PM-11-19-07 Bye... [localhost fengchengwen]# babeltrace /root/dpdk-traces/feng-2023-07-10-PM-11-19-07 | grep submit | head -1 [23:19:01.442334710] (+1.000143090) lib.dmadev.stats_get: { cpu_id = 0xA, name = "dpdk-dma" }, { dev_id = 0, vchan = 0x0, stats_submitted = 0x0, stats_completed = 0x0, stats_errors = 0x0, ret = 0 } [localhost fengchengwen]# babeltrace /root/dpdk-traces/feng-2023-07-10-PM-11-19-07 | grep dmadev.completed | head -1 [23:19:08.440327670] (+0.997219191) lib.dmadev.completed: { cpu_id = 0xB, name = "rte-worker-11" }, { dev_id = 0, vchan = 0x0, nb_cpls = 0x20, last_idx_val = 47207, has_error_val = 0, ret = 0x0 } > > Also, the test chkincs must run fine. chkincs ? > > > . >