From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
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 <dev@dpdk.org>; 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: <xms:mq3HZBJ9yEPdPNL0egM8FglY-jeh-HvXScqyTsYKVpgJ1ku8wVsVNw>
<xme:mq3HZNKISS0Ifc7Y5vM6QN0yWbde09auG4vDEIcBrSNtNoK5U9fnQ67R2tjhu9rkm
YdyVPZ3rcs3Jfd00A>
X-ME-Received: <xmr:mq3HZJtFNI6_sghy5YdqcMgysc2JmRzPW1X3C3Hb4j98b799da86SlcIN22ES_t6RVEe04TQ44n-Vc_Yly5ScRoJ8Q>
X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedviedrjeeggdeffecutefuodetggdotefrodftvf
curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu
uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc
fjughrpefhvfevufffkfgjfhgggfgtsehtqhertddttddunecuhfhrohhmpefvhhhomhgr
shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg
ftrfgrthhtvghrnhepfefhjeeluedvvedtuddtuedtvefhieejtefhffeujefhteduudev
tdektdeikeffnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrh
homhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth
X-ME-Proxy: <xmx:mq3HZCZRklxtRZ6aFR_tebkEYyfsbqRLKuDB8lF2l4da9vkIFgk4mA>
<xmx:mq3HZIZ1gN5sz9mGUgvC-UCfxUCAzIIRAlYo9Exh24ZMFwoQp5HviA>
<xmx:mq3HZGAKtzIXslBrY6mk9QrwVKtsmTXeOGevl4lCfMm81LPg-ZaRhA>
<xmx:mq3HZHzZ16OeDpMaFQAjcROAS9xGImUhtRpkfY41UXOx5lJJ_uVdTA>
Feedback-ID: i47234305:Fastmail
Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon,
31 Jul 2023 08:48:24 -0400 (EDT)
From: Thomas Monjalon <thomas@monjalon.net>
To: fengchengwen <fengchengwen@huawei.com>
Cc: dev@dpdk.org, mb@smartsharesystems.com,
Kevin Laatz <kevin.laatz@intel.com>,
Bruce Richardson <bruce.richardson@intel.com>, 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: <a41c8560-5616-42f9-bad0-8113ec13deb0@huawei.com>
References: <20230412024808.41339-1-fengchengwen@huawei.com>
<7265847.4vTCxPXJkl@thomas>
<a41c8560-5616-42f9-bad0-8113ec13deb0@huawei.com>
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=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 <fengchengwen@huawei.com>
> >>>> Acked-by: Morten Br=F8rup <mb@smartsharesystems.com>
> >>>>
> >>>> ---
> >>>> 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.