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 4566DA00C2; Fri, 24 Apr 2020 11:09:33 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 14A7C1BF80; Fri, 24 Apr 2020 11:09:32 +0200 (CEST) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id D52DE1BF67 for ; Fri, 24 Apr 2020 11:09:29 +0200 (CEST) IronPort-SDR: zZQZZPHrq5hk1tbNli2zQpoKHVVe/1AJGr8l1yT7yGlPWZjO6U16VAybdHepERZU/NTXo4W3sx lO/eTsldbDIw== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Apr 2020 02:09:28 -0700 IronPort-SDR: vzegYK0fbRfhvqitPP4wnFRXYZMjeqwE5G1JdpVbpqBWYyd83O77zV/4nonW2kKYVqa2elyRW4 YZQCpG0AjDEw== X-IronPort-AV: E=Sophos;i="5.73,311,1583222400"; d="scan'208";a="430698855" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.36.26]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 24 Apr 2020 02:09:27 -0700 Date: Fri, 24 Apr 2020 10:09:23 +0100 From: Bruce Richardson To: Lukasz Wojciechowski Cc: dev@dpdk.org Message-ID: <20200424090923.GA1440@bricha3-MOBL.ger.corp.intel.com> References: <20200417215739.23180-1-l.wojciechow@partner.samsung.com> <20200422214555.11837-1-l.wojciechow@partner.samsung.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200422214555.11837-1-l.wojciechow@partner.samsung.com> Subject: Re: [dpdk-dev] [PATCH v2 0/3] introduce global debug flag 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, Apr 22, 2020 at 11:45:52PM +0200, Lukasz Wojciechowski wrote: > This set of patches introduces a global rte_debug flag for dpdk. > This will allow easy switch to debug build configuration using a single > flag. In the debug mode a RTE_DEBUG macro is defined to 1 > and for every enabled to be built librarte a RTE_DEBUG_{library name} > is also defined. These macros can be used to place a debug code > inside #ifdef #endif clauses. > > The following requirements were discussed on the mailing list: > 1) The global debug flag is required to enable all the sanity checks > and validations that are normally not used due to performance reasons > > 2) The best option would be to have a single flag - not to introduce > too many build options > > 3) This option should be separated from meson "debug" option > (used for build with symbols) and can be called "rte_debug" > > 4) The currently existing DEBUG macros should not be replaced with > a RTE_DEBUG macro. This would allow to still enable them using > CFLAGS="-D..." to test a single module (library, driver). > > 5) Currently existing options' names should be standardized > to RTE_DEBUG_{library/driver name}, so they can be automatically enabled > when rte_debug is set. Standardized names would allow easy usage > in other modules. > > 6) The debug functionality should be encapsulated in: > if (rte_log_can_log(...)) { > ... > } > for possibility to be filtered out in runtime. > > > Because of the hot discussion of v1 version of patches, I limit > the v2 version to mbuf library changes only, to see how it will impact > the performance with rte_log_can_log usage and to get opinions. > > Next steps would be to follow changes in other libraries and drivers. > > --- > v2: > * Use new meson option rte_debug instead of debug > * Add standardized defines for built libraries > * Limit patches to mbuf library (as a POC) > * Use rte_log_can_log to wrap debug section > > Lukasz Wojciechowski (3): > config: introduce global rte debug flag > config: remove unused config flags > mbuf: standardize library debug flag > I really like this set, looks a good approach to me anyway. Out of interest - did you automate the scanning for all the different build flags to find those in the second patch that weren't used, or did you come across them by chance? Just wondering if there are any other unused flags we can remove.