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 33687A04B8; Tue, 5 May 2020 18:46:43 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 3D4AB1D66D; Tue, 5 May 2020 18:46:43 +0200 (CEST) Received: from mail-il1-f195.google.com (mail-il1-f195.google.com [209.85.166.195]) by dpdk.org (Postfix) with ESMTP id C12C21D669 for ; Tue, 5 May 2020 18:46:41 +0200 (CEST) Received: by mail-il1-f195.google.com with SMTP id w6so1129338ilg.1 for ; Tue, 05 May 2020 09:46:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=RRy+XLkNhgPkOVffHzdlCaJ/1neZ397dRzzViz2eEkk=; b=jfHMIX7syRtA+AkDvVOfEIx5d/o4YheQphEtnCi4qUZm+TAHbek9uhvi8YcM3Qegwd 6u98qxFTfC82J2rC6EJk1E43YCzAgIxggfrWjmowAp0lhlgVjGzIjeI6oQgPb+K9SiBX mrwOjgzyWXPrA0Y0yUq5a6zAsb4G2DziQUEU0wTCSFZGwh+XVVcwz2v7nNlAt4zs4DqT cTfaaWyUuPkjVhLEbUiRxfW4b2J7cVaG/k40vOASgTOtDUtHvU+wmLOMiVZED5vVH4oc 8Gs+GJ7hwNirF141wVwJppcLQfIiaDehNb8RbUp8a73h1vafi0N3ApG+chakaRbyfPgR St+g== 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=RRy+XLkNhgPkOVffHzdlCaJ/1neZ397dRzzViz2eEkk=; b=YSKSMPtdlzgCMsbuUFVlO9np6nQAs+R9036nfRIApI4DbcQ3RRHvQfwrGvM07Z0w/d AY4CAnZK/bBvKFyV6TKR93cRDqrjxwJy/nAdIdQi2G+NBhbRdLMWRN7xi4l6TYoOhkjX 88yhFp1eWR8NWSLyhP/QkMNsLApztnKzjCWc7ZDsMpaVaSdecEXbdbqdqE+UCROxVwq8 TQfQe7ojSQFP+mBUYv2VGMSD8YGlgVgleHwjEULHuKgelv8Zj9e/CNPBVMYsG154BkoI 8pT0feUYneBIszPS80VQxAi3gqFllPsoZU+zbBfGx3EYO9y6PzOHRRo9AsAedRLFhNU6 ELuQ== X-Gm-Message-State: AGi0Puba1ayV5Xs3oMaHDEIsqMQCRVn4gk41VX2mMmrQJ2LnBTJqGL2i lTymOdhZh8HuNVe4Qo65Lz78O3oECIRIJV9hqGM= X-Google-Smtp-Source: APiQypLEJS8ODXEf+/mFAovWy36IvZUxXaxfIStrAfzZVY55wGgZInsM3BfcS1g3HbtEApXCfoGb4kHJW7Ym4ifxTOM= X-Received: by 2002:a92:8b45:: with SMTP id i66mr4632760ild.162.1588697200875; Tue, 05 May 2020 09:46:40 -0700 (PDT) MIME-Version: 1.0 References: <20200503203135.6493-1-david.marchand@redhat.com> <2596990.BEx9A2HvPv@thomas> <2479551.BddDVKsqQX@thomas> In-Reply-To: From: Jerin Jacob Date: Tue, 5 May 2020 22:16:24 +0530 Message-ID: To: David Marchand Cc: Thomas Monjalon , dpdk-dev , Jerin Jacob , Sunil Kumar Kori , John McNamara , Marko Kovacevic , Declan Doherty , Ferruh Yigit , Andrew Rybchenko , Olivier Matz Content-Type: text/plain; charset="UTF-8" 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:58 PM David Marchand wrote: > > On Tue, May 5, 2020 at 5:25 PM Jerin Jacob wrote: > > > > On Tue, May 5, 2020 at 5:56 PM Jerin Jacob wrote: > > > > > > On Tue, May 5, 2020 at 5:06 PM David Marchand wrote: > > > > > > > > On Tue, May 5, 2020 at 12:13 PM Jerin Jacob wrote: > > > > > > > Please share the data. > > > > > > > > > > > > Measured time between first rte_trace_point_register and last one with > > > > > > a simple patch: > > > > > > > > > > I will try to reproduce this, once we finalize on the above synergy > > > > > with rte_log. > > > > > > > > I took the time to provide measure but you won't take the time to look at this. > > > > > > I will spend time on this. I would like to test with a shared library > > > also and more tracepoints. > > > I was looking for an agreement on using the constructor for rte_log as > > > well(Just make sure the direction is correct). > > > > > > Next steps: > > > - I will analyze the come back on this overhead on this thread. > > > > I have added 500 constructors for testing the overhead with the shared > > build and static build. > > My results inline with your results aka negligible overhead. > > > > David, > > Do you have plan for similar RTE_LOG_REGISTER as mentioned earlier? > > I would like to have rte_log and rte_trace semantics similar to registration. > > If you are not planning to submit the rte_log patch then I can send > > one for RC2 cleanup. > > It won't be possible for me. I can do that if we agree on the specifics. > > Relying on the current rte_log_register is buggy with shared builds, > as drivers are calling rte_log_register, then impose a default level > without caring about what the user passed. > So if we introduce a RTE_LOG_REGISTER macro now at least this must be fixed too. > > What I wanted to do: > - merge rte_log_register_and_pick_level() (experimental) into > rte_log_register, doing this should be fine from my pov, > - reconsider the relevance of a fallback logtype when registration fails, > - shoot the default level per component thing: levels meaning is > fragmented across the drivers/libraries because of it, but this will > open a big box of stuff, This you are referring to internal implementation improvement. Right? I was referring to remove the current clutter[1] If we stick the following as the interface. Then you can do other improvements when you get time that won't change the consumer code or interference part. #define RTE_LOG_REGISTER(type, name, level) [1] > > -/** > > - * @internal > > - */ > > -int otx2_logtype_base; > > -/** > > - * @internal > > - */ > > -int otx2_logtype_mbox; > > -/** > > - * @internal > > - */ > > -int otx2_logtype_npa; > > -/** > > - * @internal > > - */ > > -int otx2_logtype_nix; > > -/** > > - * @internal > > - */ > > -int otx2_logtype_npc; > > -/** > > - * @internal > > - */ > > -int otx2_logtype_tm; > > -/** > > - * @internal > > - */ > > -int otx2_logtype_sso; > > -/** > > - * @internal > > - */ > > -int otx2_logtype_tim; > > -/** > > - * @internal > > - */ > > -int otx2_logtype_dpi; > > -/** > > - * @internal > > - */ > > -int otx2_logtype_ep; > > - > > -RTE_INIT(otx2_log_init); > > -static void > > -otx2_log_init(void) > > -{ > > - otx2_logtype_base = rte_log_register("pmd.octeontx2.base"); > > - if (otx2_logtype_base >= 0) > > - rte_log_set_level(otx2_logtype_base, RTE_LOG_NOTICE); > > - > > - otx2_logtype_mbox = rte_log_register("pmd.octeontx2.mbox"); > > - if (otx2_logtype_mbox >= 0) > > - rte_log_set_level(otx2_logtype_mbox, RTE_LOG_NOTICE); > > - > > - otx2_logtype_npa = rte_log_register("pmd.mempool.octeontx2"); > > - if (otx2_logtype_npa >= 0) > > - rte_log_set_level(otx2_logtype_npa, RTE_LOG_NOTICE); > > - > > - otx2_logtype_nix = rte_log_register("pmd.net.octeontx2"); > > - if (otx2_logtype_nix >= 0) > > - rte_log_set_level(otx2_logtype_nix, RTE_LOG_NOTICE); > > - > > - otx2_logtype_npc = rte_log_register("pmd.net.octeontx2.flow"); > > - if (otx2_logtype_npc >= 0) > > - rte_log_set_level(otx2_logtype_npc, RTE_LOG_NOTICE); > > - > > - otx2_logtype_tm = rte_log_register("pmd.net.octeontx2.tm"); > > - if (otx2_logtype_tm >= 0) > > - rte_log_set_level(otx2_logtype_tm, RTE_LOG_NOTICE); > > - > > - otx2_logtype_sso = rte_log_register("pmd.event.octeontx2"); > > - if (otx2_logtype_sso >= 0) > > - rte_log_set_level(otx2_logtype_sso, RTE_LOG_NOTICE); > > - > > - otx2_logtype_tim = rte_log_register("pmd.event.octeontx2.timer"); > > - if (otx2_logtype_tim >= 0) > > - rte_log_set_level(otx2_logtype_tim, RTE_LOG_NOTICE); > > - > > - otx2_logtype_dpi = rte_log_register("pmd.raw.octeontx2.dpi"); > > - if (otx2_logtype_dpi >= 0) > > - rte_log_set_level(otx2_logtype_dpi, RTE_LOG_NOTICE); > > - > > - otx2_logtype_ep = rte_log_register("pmd.raw.octeontx2.ep"); > > - if (otx2_logtype_ep >= 0) > > - rte_log_set_level(otx2_logtype_ep, RTE_LOG_NOTICE); > > - > > -} > > +RTE_LOG_REGISTER(otx2_logtype_base, pmd.octeontx2.base, NOTICE); > > +RTE_LOG_REGISTER(otx2_logtype_mbox, pmd.octeontx2.mbox, NOTICE); > > +RTE_LOG_REGISTER(otx2_logtype_npa, pmd.mempool.octeontx2, NOTICE); > > +RTE_LOG_REGISTER(otx2_logtype_nix, pmd.net.octeontx2, NOTICE); > > +RTE_LOG_REGISTER(otx2_logtype_npc, pmd.net.octeontx2.flow, NOTICE); > > +RTE_LOG_REGISTER(otx2_logtype_tm, pmd.net.octeontx2.tm, NOTICE); > > +RTE_LOG_REGISTER(otx2_logtype_sso, pmd.event.octeontx2, NOTICE); > > +RTE_LOG_REGISTER(otx2_logtype_tim, pmd.event.octeontx2.timer, NOTICE); > > +RTE_LOG_REGISTER(otx2_logtype_dpi, pmd.raw.octeontx2.dpi, NOTICE); > > +RTE_LOG_REGISTER(otx2_logtype_ep, pmd.raw.octeontx2.ep, NOTICE); > > > > diff --git a/lib/librte_eal/include/rte_log.h b/lib/librte_eal/include/rte_log.h > > index 1789ede56..22fc3802f 100644 > > --- a/lib/librte_eal/include/rte_log.h > > +++ b/lib/librte_eal/include/rte_log.h > > @@ -376,6 +376,15 @@ int rte_vlog(uint32_t level, uint32_t logtype, > > const char *format, va_list ap) > > RTE_LOGTYPE_ ## t, # t ": " __VA_ARGS__) : \ > > 0) > > > > +#define RTE_LOG_REGISTER(type, name, level) \ > > +int type; \ > > +RTE_INIT(__##type) \ > > +{ \ > > + type = rte_log_register(RTE_STR(name)); \ > > + if (type >= 0) \ > > + rte_log_set_level(type, RTE_LOG_##level); \ > > +} > > + > > > > > -- > David Marchand >