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 70876A0350; Wed, 24 Jun 2020 20:10:32 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 507831D9BF; Wed, 24 Jun 2020 20:10:32 +0200 (CEST) Received: from mail-io1-f66.google.com (mail-io1-f66.google.com [209.85.166.66]) by dpdk.org (Postfix) with ESMTP id 205471D936 for ; Wed, 24 Jun 2020 20:10:30 +0200 (CEST) Received: by mail-io1-f66.google.com with SMTP id a12so3193078ion.13 for ; Wed, 24 Jun 2020 11:10:30 -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=RB8+SwFQFAhVKciicsxRrhr7SMW1DJw6hWBwiJv9XVQ=; b=XTPaHhpqmL/p3O82O8HwbdrfhW3z0y+DATPObA3MOfIRyNVvR2eNhTxIm4PYrOIzNC KmudZmQoT69d4BdVVlXH2zqkYPYm7RByCEzzx7bHPoQIRKeewgBY+a1RqHoT359OjaKT yDxrulJqOCumQMQLSbIqSu2NNrBsq9J4uRS7Me09+Tq2FcYoxvR4F383kvHCVRJ3AAG/ mqJ4l11UtGsFTisU+YJ+1oAKFLkWA6pdlXdEgZwTLAlvmVnuKCFEsHcQNUlR21csbN7w ibeMw1/qbg16QTQ1qEeNOOz3Rg37fujE/qDX5noWeKwKiFqAU4eaee0iWfo2x3Cbm6nn Ee5w== 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=RB8+SwFQFAhVKciicsxRrhr7SMW1DJw6hWBwiJv9XVQ=; b=PvIEjnRTAxB8z6NIOntGyGXtVfBV6UQR6Ysbr7qldrn7r6C7HswhMzK1JoeH8HQ7O+ 1RDN8EfAutNlAkxBdV/WK4E85kqlXAcUrVz9RLGCUAtRYrjP0pgjNIbPItZQY6pL+YTG MGbrVbbR6m6046oWhy1ECZZFwxXFYVMIzUmv/ZlsV1Q2z+ymW+RkG6CB8+KW9K3QP41a 1pRF0ul3QU9wpchjBRrIv6bHpzDZhAXhqDGGyVJtHZCCM279Qk6vTrNXFNhKcgV9kUpQ rb2/ZSGRZA98tAyuk4gdMLf4szvrzrSrFVkPphytO4FtmYzvp1UtuOIblNrLb1h7n4kB yWYw== X-Gm-Message-State: AOAM532//GmnLH6uXtLMzEOrg04fIFpNmZyekmQbZzLAUG+ObPbN0Scf MAMZ431hndOwjFiP/jw2GbzDXo6bdNILsXWn/p8= X-Google-Smtp-Source: ABdhPJz9XLGNrytfYzIhsApil60bf10UZZtT3qD/l56fmG5SNGwaNDET81qdw2414mztZZ9CSpGRijHoy8VupWwJ5Iw= X-Received: by 2002:a02:cb59:: with SMTP id k25mr15074072jap.112.1593022229319; Wed, 24 Jun 2020 11:10:29 -0700 (PDT) MIME-Version: 1.0 References: <20200617063047.1555518-1-jerinj@marvell.com> <20200617063047.1555518-2-jerinj@marvell.com> <4becf100-7f0f-d051-a40c-3944e101381a@solarflare.com> <11e13bf9-8400-50de-4638-cdd1286915e4@solarflare.com> In-Reply-To: <11e13bf9-8400-50de-4638-cdd1286915e4@solarflare.com> From: Jerin Jacob Date: Wed, 24 Jun 2020 23:40:13 +0530 Message-ID: To: Andrew Rybchenko Cc: David Marchand , Jerin Jacob Kollanukkaran , dev , Thomas Monjalon , Olivier Matz Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH 01/13] eal/log: introduce log register macro 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 Wed, Jun 24, 2020 at 9:13 PM Andrew Rybchenko wrote: > > On 6/24/20 6:32 PM, Jerin Jacob wrote: > > On Wed, Jun 24, 2020 at 8:56 PM Andrew Rybchenko > > wrote: > >> > >> On 6/24/20 4:11 PM, Jerin Jacob wrote: > >>> On Wed, Jun 17, 2020 at 3:51 PM Andrew Rybchenko > >>> wrote: > >>>> > >>>> On 6/17/20 1:02 PM, David Marchand wrote: > >>>>> On Wed, Jun 17, 2020 at 8:30 AM wrote: > >>>>>> > >>>>>> From: Jerin Jacob > >>>>>> > >>>>>> Introducing the RTE_LOG_REGISTER macro to avoid the code duplication > >>>>>> in the log registration process. > >>>>>> > >>>>>> It is a wrapper macro for declaring the logtype, register the log and sets > >>>>> > >>>>> Having the logtype variable declared as part of the macro will force > >>>>> us to have global symbols (for the cases where it is needed). > >>>>> I'd rather leave the declaration to the caller, and only handle the > >>>>> registering part in this macro. > >>>> > >>>> I agree with David that it is important to avoid global symbols > >>>> when it is not needed. > >>> > >>> David, Andrew, > >>> > >>> Since it is executed in "constructor" context, it will be always from > >>> the global variable. Right? > >>> i.e DPDK is not yet initialized in when "constructor" being called. > >>> In addition to that, It will be adding more lines of code in the > >>> consumer of this MACRO. > >>> Thoughts? > >> > >> The problem is rather 'extern' vs 'static'. Before the patch > >> many variables are static, but become externally visible after > >> the patch. > > > > OK. How about RTE_LOG_REGISTER_EXTERN or RTE_LOG_REGISTER_STATIC then? > > It will allow less code in the consumer of this macro. > > May be default we an make it as static so RTE_LOG_REGISTER and > > RTE_LOG_REGISTER_EXTERN > > for the different needs. > > > > Thoughts? > > Yes, I think it is a possible solution to use static in > RTE_LOG_REGISTER and use RTE_LOG_REGISTER_EXTERN > for non-static version. If we go this way, I'd prefer > the option. OK. > > Alternative is to keep variable declaration outside, > as David suggested, and I tend to agree that it is a > bit better. Macro name says 'register'. It is not > 'declare and register'. Also it avoids static-vs-extern > problem completely. The solution allows to keep the > variable declaration untouched and put constructor (macro) > at the end of fine where constructors typically reside. My only concern with that approach is that, We can not save a lot of code duplication with that scheme. ie. it is [1] vs [2]. We can change the MACRO name accordingly if that is a concern. Any suggestions? Let me know your preference on [1] vs [2], I will stick with for the next version. [1] 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) [2] int otx2_logtype_base; int otx2_logtype_mbox; int otx2_logtype_npa; int otx2_logtype_nix; int otx2_logtype_npc; int otx2_logtype_tm; int otx2_logtype_sso; int otx2_logtype_tim; int otx2_logtype_dpi; int otx2_logtype_ep; 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)