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 DBE61A0540; Mon, 13 Jul 2020 11:04:18 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 491171D546; Mon, 13 Jul 2020 11:04:18 +0200 (CEST) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 210DA1D52B for ; Mon, 13 Jul 2020 11:04:15 +0200 (CEST) IronPort-SDR: IRyQGwl+a2L+XwwosqdVf9PKWkmL3/NO0TbVLiBsQC1H84QqBktSlb3uw/7b2YKhdTIibQIWM/ xKxc6wQDJFRg== X-IronPort-AV: E=McAfee;i="6000,8403,9680"; a="148576621" X-IronPort-AV: E=Sophos;i="5.75,347,1589266800"; d="scan'208";a="148576621" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Jul 2020 02:04:15 -0700 IronPort-SDR: wKin40skwbkGqb6Orii2j+oKlTgunE7dL5/oJfsFjSaSSpzbSRd66ym341WXbw5wQU54isvV7Y iXwItDGIGbpw== X-IronPort-AV: E=Sophos;i="5.75,347,1589266800"; d="scan'208";a="459224902" Received: from bricha3-mobl.ger.corp.intel.com ([10.249.32.149]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 13 Jul 2020 02:04:13 -0700 Date: Mon, 13 Jul 2020 10:04:09 +0100 From: Bruce Richardson To: Thomas Monjalon Cc: Lukasz Wojciechowski , dev@dpdk.org, david.marchand@redhat.com Message-ID: <20200713090409.GA694@bricha3-MOBL.ger.corp.intel.com> References: <20200422214555.11837-1-l.wojciechow@partner.samsung.com> <20200709134823.9176-1-l.wojciechow@partner.samsung.com> <2283396.fTMGzKxhym@thomas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2283396.fTMGzKxhym@thomas> Subject: Re: [dpdk-dev] [PATCH v3 0/4] 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 Sat, Jul 11, 2020 at 05:11:52PM +0200, Thomas Monjalon wrote: > 09/07/2020 15:48, Lukasz Wojciechowski: > > 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 library a RTE_DEBUG_{library name} > > and for every enabled to be built driver > > a RTE_DEBUG_{driver_class}_{driver_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). > > Please can we clarify this point? > You mean not replacing driver macros with the global RTE_DEBUG? > But we agree (next point) to replace existing macros? > > > 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. > > > > v3 contains mbuf performance tests, which might help dpdk developers > > community to decide if drop of performance related to rte_log_can_log > > can be accepted. > > > > If agreement is reached, next steps would be to follow changes > > in other libraries and drivers. > > If I understand well, it makes no sense to apply this v3, > without having the rest of the code converted? > I'd nearly take the opposite view, that merging this patchset is a pre-requisite to converting the rest of the code. By merging this now, it allows others to work in parallel on any conversion they want to do, rather than requiring it all to be part of the one patchset. On the other hand, it would be good to get more buy-in from maintainers that this does meet their requirements at this point (and if not what could be changed to make it meet them) Regards, /Bruce