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 318BAA0350; Tue, 30 Jun 2020 15:39:46 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 0F5F91BF7C; Tue, 30 Jun 2020 15:39:45 +0200 (CEST) Received: from us-smtp-delivery-1.mimecast.com (us-smtp-2.mimecast.com [207.211.31.81]) by dpdk.org (Postfix) with ESMTP id 598431BF70 for ; Tue, 30 Jun 2020 15:39:43 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1593524382; 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: in-reply-to:in-reply-to:references:references; bh=PcPyEfKa2rd+HPATkyvNnBMcaswgBBcXSSbZRi3QkpM=; b=Y+m+7MRfXTpOt4Ac5yirNmCkGunDOvvO4haqULNK7b21SW5d+6t739pokwLkiz7XxoqRYi vFiMHSZS/iE374Z8lJU/HWu4I2quqLU2dr+8BsDSmAV3ut8lLEhpA68wvsfdLhvbNm4pjh szR5zzwyrAsU5sLIKEscvo+IvckOTcs= 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-271-xiH8Z9JMNsqr6acdBI6YQg-1; Tue, 30 Jun 2020 09:39:40 -0400 X-MC-Unique: xiH8Z9JMNsqr6acdBI6YQg-1 Received: by mail-vs1-f70.google.com with SMTP id n124so5953200vsc.23 for ; Tue, 30 Jun 2020 06:39:40 -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=PcPyEfKa2rd+HPATkyvNnBMcaswgBBcXSSbZRi3QkpM=; b=k7NUZIUggOgBH4Uz459XjAbXVZDIaHjHo/jU2pSPqgJQwnHDwSczLrW1XokZ6/d3z/ 1DE04P4Ay3S/ZsN2YpF1jBKbLyOlAG29rNyR9tt0jUE3TMZzpEbWVPMXLkPT8sq6tQbR RgB45Lg8dvHiYBQtJpZ/WeLY4M/EbQ18+Per8j8BBNJ8TST1KqbXLcttTr8+VfVu5f/w Bj41FjKv6DqwGLtsmfJ3bDlhcqcNjA8Iu3bUWn/M9SvdquEFVrNQW2NMpbTcg3AxTVle A2twSDfU05qW0zhhsvp0uFv053kg6gchAnFdvRVPBKqXQajFhbgy6D/vdVzONTISp97x AVNw== X-Gm-Message-State: AOAM530cldCjbfbk/Q1Wc+GZecqUD8nH9n6QGy/FjH/AdnLHhclYoo+U wAsAKWKqTLvIDLwoq0ogR0De4Kvn28H4TYE/iRknSldNgUFJyJDtWsm1Vqnl3vOgNj0esZGdOPl x5ILiZ2Y6Wj46F5PKd8I= X-Received: by 2002:a67:c58c:: with SMTP id h12mr7795418vsk.141.1593524378831; Tue, 30 Jun 2020 06:39:38 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzmA4PzVPFV95zOmxZx6Vwu/EjRr2k6qhiPpd4lj96gcDe9EXaYGvmJhcbSSY7Ku/SBybjBT9D4GGZW9b9eZxw= X-Received: by 2002:a67:c58c:: with SMTP id h12mr7795280vsk.141.1593524376937; Tue, 30 Jun 2020 06:39:36 -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: David Marchand Date: Tue, 30 Jun 2020 15:39:25 +0200 Message-ID: To: Jerin Jacob Cc: Andrew Rybchenko , Jerin Jacob Kollanukkaran , dev , Thomas Monjalon , Olivier Matz Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dmarchan@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com 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 Fri, Jun 26, 2020 at 2:38 PM Jerin Jacob wrote: > > Or no declaration in macro and leave code as it is. > > I dont see anything wrong with that. Do you have technical rationale > for the above rule? Avoid global symbols when unneeded. Global symbols can be hidden with shared build, but that's not the case with static build. This is why we try to maintain a namespace for DPDK api. For logtypes, a lot of those are already global and nobody complained about them (lucky ?). With this patch, the static ones, that we would convert to global symbols, are the following: drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c:static int fpga_5gnr_fec_logtype; drivers/baseband/fpga_lte_fec/fpga_lte_fec.c:static int fpga_lte_fec_logtype; drivers/baseband/null/bbdev_null.c:static int bbdev_null_logtype; drivers/baseband/turbo_sw/bbdev_turbo_software.c:static int bbdev_turbo_sw_logtype; drivers/net/af_packet/rte_eth_af_packet.c:static int af_packet_logtype; drivers/net/af_xdp/rte_eth_af_xdp.c:static int af_xdp_logtype; drivers/net/kni/rte_eth_kni.c:static int eth_kni_logtype; drivers/net/null/rte_eth_null.c:static int eth_null_logtype; drivers/net/pcap/rte_eth_pcap.c:static int eth_pcap_logtype; drivers/net/ring/rte_eth_ring.c:static int eth_ring_logtype; drivers/net/softnic/rte_eth_softnic.c:static int pmd_softnic_logtype; drivers/net/vdev_netvsc/vdev_netvsc.c:static int vdev_netvsc_logtype; drivers/net/vhost/rte_eth_vhost.c:static int vhost_logtype; drivers/vdpa/ifc/ifcvf_vdpa.c:static int ifcvf_vdpa_logtype; lib/librte_bbdev/rte_bbdev.c:static int bbdev_logtype; lib/librte_cfgfile/rte_cfgfile.c:static int cfgfile_logtype; lib/librte_eventdev/rte_event_timer_adapter.c:static int evtim_logtype; lib/librte_eventdev/rte_event_timer_adapter.c:static int evtim_buffer_logtype; lib/librte_eventdev/rte_event_timer_adapter.c:static int evtim_svc_logtype; lib/librte_pdump/rte_pdump.c:static int pdump_logtype; All of them follow some kind of convention of finishing with _logtype. Is this enough and we won't trigger link issues in existing applications? Probably not possible to tell. There was a similar discussion with Gaetan in the pci bus code, and so far we are still in this status quo for global symbols. So ok, let's go with this embedded global symbol. If we hit an issue with static linking, we will need a tree-wide cleanup. > > > > - Having components set log levels at init time in the macro is a bug to me. > > > > This has been worked around with > > > > rte_log_register/rte_log_register_and_pick_level but the initial > > > > problem is that rte_log_set_level* should only be called by the user. > > > > > > I agree with the below stuff, That's is not introduced by this patch. > > > It is already there. > > > Be it macro or no macro code. > > > > > > I think this patch helps to change to new scheme as it takes care of > > > changing the > > > registration part to commonplace so that we can set to the same level > > > in the future if > > > everyone agrees to it > > > > We will still expose this macro meaning that we will have an API breakage later. > > So if we go with introducing this, let's make it good from the start. > > But Is everyone OK with removing the "level" from the register before > we think about > breaking the API later?. I see PMD uses multiple levels in the same > PMD for different > purposes. It is an API fix to me, as there should be no place for "level" interpretation. Just a note, a v2 would be necessary for the rte_log_register_type_and_pick_level use in any case. -- David Marchand