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 581C9A0350; Fri, 26 Jun 2020 13:16:19 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id C79BD1BEC5; Fri, 26 Jun 2020 13:16:18 +0200 (CEST) Received: from mail-il1-f193.google.com (mail-il1-f193.google.com [209.85.166.193]) by dpdk.org (Postfix) with ESMTP id 853591BEA9 for ; Fri, 26 Jun 2020 13:16:17 +0200 (CEST) Received: by mail-il1-f193.google.com with SMTP id x18so8139505ilp.1 for ; Fri, 26 Jun 2020 04:16:17 -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=hfe+B4GZQ3QqFUveRSFPFj09jzZZmZyyUZIullE06Fs=; b=VWqDxWkDTwCHKJvDwTEBkOO8buTxjuipHk1UHBsKQbF9Db5sERR9tXPAujoCUqLq7b by1DeP+hOmvAqfWnRvGbAhitr6xHhJLpKse62cVLsgqDaNvUBNGBw0tRZI+1Hw50l/W5 B8iD+vb8RvtVqm9A268rlYsNwbWFy++HmRKBXsbBiOYESkIXT7Pdkx68hbZoaiIILElf 0jV8aHNt4nP+xCrcaHONP0LSXHa6JKzsk+WfG9zZDUnyWzQweZxGQ305qtErhM7yTW/A fuFdy2eJM+8q/e5e2g8rQ+4qKbu4wGojqNVtYCYRB8xinVpIIE1rWcL3x34R2P6I1qAI Uqsw== 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=hfe+B4GZQ3QqFUveRSFPFj09jzZZmZyyUZIullE06Fs=; b=HnJCvD2QdrnE9JDKpkxkzak/QS285v/Ypctq8pzuWPSMh2xTUnDzzJAAcvHIJtiMDV a6KO8b4Pq3RzesGzpAPLcNtyPMj/VkUFmSGlGlImmztPIquv/kaHgteJmLQKhlnqBUw6 OhfFtVlvWqmJq9gl+gPqwGMy1n8ZmF6FKLZWGt+iB2MHlzjuEaKjQliMn2vIE7g2GhwX HXDXN6gz2CN+yKAclsUGevwwVEIgtxTjWS7z2z8Zi9zsnvm6bQHzOokR+whcqMNo70EL fwmnpSgkxYYPXXWM91aQBNP0OENrH0MzTU6Ok3Twd4uxJHWSVyygwRe6nZezez3q5BXu abFw== X-Gm-Message-State: AOAM5304wU4WbEgE61miQ7B05teEjjdTRnF17t9uOIqAT3eYzoHg3MfF FsFTJ0ijtjx/sj4k8c8JJbD9Ps1hbtDxUCSrSJM= X-Google-Smtp-Source: ABdhPJxf9hnf4/GYOOOoSZ7VPouQCB7BW3k8W60UPQ1HCxGsRRkYhtff6SKTkDhVQMcTsc37OTc0u7S52h6nAkJ7+c0= X-Received: by 2002:a92:d01:: with SMTP id 1mr1694040iln.294.1593170176646; Fri, 26 Jun 2020 04:16:16 -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: From: Jerin Jacob Date: Fri, 26 Jun 2020 16:46:00 +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 11:40 PM Jerin Jacob wrote: > > 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. If there are no other comments, I change RTE_LOG_REGISTER to static version and RTE_LOG_REGISTER_EXTERN for a non-static version and send 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)