From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 9E3A9A04B6; Tue, 5 May 2020 10:23:39 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 4ED441D159; Tue, 5 May 2020 10:23:39 +0200 (CEST) Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.120]) by dpdk.org (Postfix) with ESMTP id F1BB71D153 for ; Tue, 5 May 2020 10:23:37 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1588667017; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=sIKopcNpuro9pyQBnaDmzghQaCTfxGQHYJXpMMFZVO0=; b=cT6ynejCnlFPWqsazDACBSiCJbvYQHXVhyFfKzj/3zH1odXmJCasB8lKpOj28C3JUayjbj B8T8+PK3dV/9I3BDXoHhU5W4dJshsGKJYq9bQW8UDi3MJ3kVMMuah/19kVqjAqYIhXFbWj k45noskSY9fQuOVK/D1EQtT7bj77q4M= Received: from mail-vs1-f70.google.com (mail-vs1-f70.google.com [209.85.217.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-421-43DWt6XRNJqwxwGBBAM3nw-1; Tue, 05 May 2020 04:23:32 -0400 X-MC-Unique: 43DWt6XRNJqwxwGBBAM3nw-1 Received: by mail-vs1-f70.google.com with SMTP id a4so270975vsp.8 for ; Tue, 05 May 2020 01:23:32 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=bK18T9ryBxqCoRRwWuH89PBLvWoLxQX6JFMjC2KOlPQ=; b=ZFx0a3f2/K4lxnozPhEFqFsi6DytQJq0iD0cMSi6b7efKZH2doZxGIB6XSPorwEfPM c4KsAGBCmzuXr43k6KW0CpDKeD11RmaMFaZW/aFqgB6953WUX4NAxf2Aeb2y7uF7bYjG /ObDYja8a6OIQtpVy01vMt2y7hTREeYAOHeq9wajCd3zNpykxqwVDaw8rYSeUUmgFTkN 5riI2E8GvnQJFcYFGXH+ygk7exBmtRlbtz4o+dDC31CeShSKWVwLgcUwlgBsbGaDr+Zv M4JOX/q41Is+3z/D287TpBFJd+FdVPTaFpyH60zDnUeTRR5n7s+WEUsPwp2U8qSjT3Hu EPLQ== X-Gm-Message-State: AGi0PuZGLgEvXzUZl9x4JmwI88Ip0TFQb6kRN88FBt+ofNwlDHz5nUwu Sd50DVxyHEXu9aQQq6A6Y1lGSypWf9WpHxQ/WAaRLVILY94iQOCsOVZIh0EQoNerO6OHVzGQ9R9 LTAIqUA1RvNTuBDnXzfQ= X-Received: by 2002:a05:6102:382:: with SMTP id m2mr1883803vsq.141.1588667011876; Tue, 05 May 2020 01:23:31 -0700 (PDT) X-Google-Smtp-Source: APiQypJHwfy9g+XNWGJ09o7NMa9u8puaB4JWQEqvyviKf9gEJNt7XqBhslcQIU1tYRR122BhufApJ3TCqyXmKB/wje8= X-Received: by 2002:a05:6102:382:: with SMTP id m2mr1883778vsq.141.1588667011407; Tue, 05 May 2020 01:23:31 -0700 (PDT) MIME-Version: 1.0 References: <20200503203135.6493-1-david.marchand@redhat.com> <2596990.BEx9A2HvPv@thomas> <2479551.BddDVKsqQX@thomas> In-Reply-To: From: David Marchand Date: Tue, 5 May 2020 10:23:19 +0200 Message-ID: To: Jerin Jacob Cc: Thomas Monjalon , dpdk-dev , Jerin Jacob , Sunil Kumar Kori , John McNamara , Marko Kovacevic , Declan Doherty , Ferruh Yigit , Andrew Rybchenko , Olivier Matz X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH 2/8] trace: simplify trace point registration X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Tue, May 5, 2020 at 9:33 AM Jerin Jacob wrote: > > > What the proposed patch here. > > > # Making N constructors from one > > > # Grouping global variable and register function under a single Marco > > > and making it as N constructors. > > > Why can we do the same logic for rte_log? > > > > rte_log is simple, there is nothing to simplify. > > Why not make, rte_log_register() and the global variable under a macro? > That's something done by the proposed patch. At the moment, there is not much that would go into such a macro, but I had started to do some cleanups on logtype registration. I did not finish because the question of the default log level is still unclear (and I did not take the time). Having the logtype definition as part of the macro would be fine to me. https://patchwork.dpdk.org/patch/57743/ > > > > rte_trace requires 3 macros calls per trace type: > > > > RTE_TRACE_POINT_REGISTER, RTE_TRACE_POINT_DEFINE, RTE_TRACE_POINT_A= RGS > > > > This patch is unifying the first 2 macro calls to make usage simple= r, > > > > and ease rte_trace adoption. > > > > > > RTE_TRACE_POINT_ARGS is NOP and for the syntax. > > > It is similar to rte_log. rte_log don't have RTE_TRACE_POINT_REGISTER= instead > > > it is creating global variable see, "int otx2_logtype_base; > > > > > > > > > > > Note: the other usability weirdness is mandating declaring each tra= ce > > > > function with a magic double underscore prefix which appears nowher= e else. > > > > > > > > > > > > > Analyze the impact wrt boot time and cross-platform pov as the lo= g > > > > > has a lot of entries to test. If the usage makes sense then it sh= ould make sense > > > > > for rte_log too. I would like to NOT have trace to be the first > > > > > library to explode > > > > > with the constructor scheme. I suggest removing this specific pat= ch from RC2 and > > > > > revisit later. > > > > > > > > You still did not give any argument against increasing the number > > > > of constructors. > > > > > > If you are proposing the new scheme, you have to prove the overhead > > > with a significant number of constructors > > > and why it has differed from existing scheme of things. That's is the > > > norm in opensource. > > > > I say there is no overhead. > > Please share the data. Measured time between first rte_trace_point_register and last one with a simple patch: diff --git a/lib/librte_eal/common/eal_common_trace.c b/lib/librte_eal/common/eal_common_trace.c index 875553d7e..95618314b 100644 --- a/lib/librte_eal/common/eal_common_trace.c +++ b/lib/librte_eal/common/eal_common_trace.c @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -23,6 +24,9 @@ static RTE_DEFINE_PER_LCORE(int, ctf_count); static struct trace_point_head tp_list =3D STAILQ_HEAD_INITIALIZER(tp_list= ); static struct trace trace =3D { .args =3D STAILQ_HEAD_INITIALIZER(trace.ar= gs), }; +uint64_t first_register; +uint64_t last_register; + struct trace * trace_obj_get(void) { @@ -43,6 +47,8 @@ eal_trace_init(void) /* Trace memory should start with 8B aligned for natural alignment = */ RTE_BUILD_BUG_ON((offsetof(struct __rte_trace_header, mem) % 8) != =3D 0); + trace_err("delta=3D%"PRIu64, last_register - first_register); + /* One of the trace point registration failed */ if (trace.register_errno) { rte_errno =3D trace.register_errno; @@ -425,6 +431,9 @@ __rte_trace_point_register(rte_trace_point_t *handle, const char *name, goto fail; } + if (first_register =3D=3D 0) + first_register =3D rte_get_tsc_cycles(); + /* Check the size of the trace point object */ RTE_PER_LCORE(trace_point_sz) =3D 0; RTE_PER_LCORE(ctf_count) =3D 0; @@ -486,6 +495,8 @@ __rte_trace_point_register(rte_trace_point_t *handle, const char *name, STAILQ_INSERT_TAIL(&tp_list, tp, next); __atomic_thread_fence(__ATOMIC_RELEASE); + last_register =3D rte_get_tsc_cycles(); + /* All Good !!! */ return 0; free: I started testpmd 100 times for static and shared gcc builds (test-meson-builds.sh) on a system with a 2.6GHz xeon. v20.05-rc1-13-g08dd97130 (before patch): static: count=3D100, min=3D580812, max=3D1482326, avg=3D1764932 shared: count=3D100, min=3D554648, max=3D1344163, avg=3D1704638 v20.05-rc1-14-g44250f392 (after patch): static: count=3D100, min=3D668273, max=3D1530330, avg=3D1682548 shared: count=3D100, min=3D554634, max=3D1330264, avg=3D1672398 --=20 David Marchand