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 89BB042FA3; Mon, 31 Jul 2023 14:48:29 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7468240A89; Mon, 31 Jul 2023 14:48:29 +0200 (CEST) Received: from wout4-smtp.messagingengine.com (wout4-smtp.messagingengine.com [64.147.123.20]) by mails.dpdk.org (Postfix) with ESMTP id 6B7714067B for ; Mon, 31 Jul 2023 14:48:28 +0200 (CEST) Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.west.internal (Postfix) with ESMTP id 0ADCC320090A; Mon, 31 Jul 2023 08:48:26 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute3.internal (MEProxy); Mon, 31 Jul 2023 08:48:27 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:cc:content-transfer-encoding:content-type:content-type:date :date:from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:sender:subject:subject:to:to; s=fm3; t= 1690807706; x=1690894106; bh=ocUMv0Y9YAStJClcJt9bW7+AhhHi4Cfx/L7 KSBfx4d8=; b=OM/cZPSgUwqU/lZcrElul7iXRcw7wPswj+6g2wwb8BcaZN/5TMN mKH4YMVOEGLxxvRSzBzxaoaF/3wjwuWSsRN8QIYEKkSJsalM3gq2dnR6eBg0KqtQ w1bmS6n3+Fxn1X/6HpwI9nFUlEKuK5/4r2NP8OwUu0VHJLbtA7clktXGYhMmUCEW zlThKQYxtSeinlkSmrCewEgjq2d2HYVb2ZrNKEcMvSu+c5HxJ2Ezn/UOs2TSu77z rsmN2iUp5yR+epKa20ni9KCPcXWchHUwWYEaHyKB/sRUNiIvZzgbPlqp09+Y/Wkd CwCWs7CVuYNvdUm0578iyD0UAU4XfzllRcQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:sender:subject:subject:to:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm3; t= 1690807706; x=1690894106; bh=ocUMv0Y9YAStJClcJt9bW7+AhhHi4Cfx/L7 KSBfx4d8=; b=p43UqRBf1sj3geX39/Ucf2dPZr/k/nefgEHsys9aQNeUH03jH1X CwIaOZG4nkuEf2Xj57D5UmxosT38zbA+TGe7IJOfsMNsKcX6dTpc3UEtsKof/OPI IPRIIXqPuTacgtnhtKav0vlCmhwG2Y7vMTdNYkX1sdVzd1vIpTMq8EkhZVJJn2Iz TKmQkVBAoA+FxJV16MBE/NHtNQmdt9fOkYCEEIxd2UoJswmFKYZvDHkUv+pDSlQw FPP9XVs3qhu7JT4g9uGgL66Oc2YmDAgzOXNQ8pNmtGkRDqxz/kysCDdOPOVGUSwZ YpkginVlNnMrehnT7GOjQY2KpP3RhoWNmvg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedviedrjeeggdeffecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvfevufffkfgjfhgggfgtsehtqhertddttddunecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg ftrfgrthhtvghrnhepfefhjeeluedvvedtuddtuedtvefhieejtefhffeujefhteduudev tdektdeikeffnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrh homhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: Feedback-ID: i47234305:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 31 Jul 2023 08:48:24 -0400 (EDT) From: Thomas Monjalon To: fengchengwen Cc: dev@dpdk.org, mb@smartsharesystems.com, Kevin Laatz , Bruce Richardson , dev@dpdk.org, david.marchand@redhat.com Subject: Re: [PATCH v4] dmadev: add tracepoints Date: Mon, 31 Jul 2023 14:48:23 +0200 Message-ID: <1737546.vCJZsxu672@thomas> In-Reply-To: References: <20230412024808.41339-1-fengchengwen@huawei.com> <7265847.4vTCxPXJkl@thomas> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="iso-8859-1" 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 10/07/2023 09:50, fengchengwen: > Hi Thomas, >=20 > 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=F8rup > >>>> > >>>> --- > >>>> 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_cp= ls, > >> 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_err= or), only record > >> the pointer value (i.e. address value). > >> > >> I think the pointer value has no mean (in particular, many of there po= inters 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 =3D *has_error; // pointer reference > >> int last_idx_val =3D *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 poi= nter > >> rte_trace_point_emit_int(has_error_val); // record the value of poi= nter > >> 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 undefin= ed. > >> > >> > >> To solve this problem, consider the rte_dmadev_trace_points.c will inc= lude rte_trace_point_register.h, > >> and the rte_trace_point_register.h will defined macro: _RTE_TRACE_POIN= T_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 =3D 0; > >> bool __has_error =3D false; > >> last_idx =3D &__last_idx; // make sure the pointer h= as meaningful value. > >> has_error =3D &__has_error; // so that the next pointe= r reference will work well. > >> #endif /* _RTE_TRACE_POINT_REGISTER_H_ */ > >> int has_error_val =3D *has_error; > >> int last_idx_val =3D *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=3Dtrue. > >> > >> ./dpdk-test --file-prefix=3Dfeng123 --trace=3Dlib.dmadev.* -l 10-11 > >=20 > > This is the test application, not the example. > > Please make sure examples/dma/ is compiling. >=20 > Work well with examples/dma (compiled with enable_trace_fp=3Dtrue). Can you try with enable_trace_fp=3Dfalse (the default)? > > Also, the test chkincs must run fine. >=20 > chkincs ? If this a word you don't know, you can try "git grep" to better understand. There is a Meson option "check_includes" to enable chkincs. I recommend using devtools/test-meson-builds.sh to test patches, it includes the above options.