From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 1C5845A83 for ; Tue, 3 Mar 2015 14:31:52 +0100 (CET) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga103.fm.intel.com with ESMTP; 03 Mar 2015 05:26:57 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.09,681,1418112000"; d="scan'208";a="693263126" Received: from irsmsx153.ger.corp.intel.com ([163.33.192.75]) by orsmga002.jf.intel.com with ESMTP; 03 Mar 2015 05:31:50 -0800 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.117]) by IRSMSX153.ger.corp.intel.com ([169.254.9.234]) with mapi id 14.03.0195.001; Tue, 3 Mar 2015 13:31:49 +0000 From: "Ananyev, Konstantin" To: "Richardson, Bruce" , Marc Sune Thread-Topic: [dpdk-dev] [PATCH] mk: add support for gdb debug info generation Thread-Index: AQHQTpX7K/KSn4KDi0q/6hBZ4HLQyZ0JgHeAgAEMsoCAAC5AgIAABfAAgAAEXYCAAAHfgIAABshw Date: Tue, 3 Mar 2015 13:31:48 +0000 Message-ID: <2601191342CEEE43887BDE71AB977258213F38C9@irsmsx105.ger.corp.intel.com> References: <1396546285-29972-1-git-send-email-cchemparathy@tilera.com> <2601191342CEEE43887BDE71AB9772580EF948F7@IRSMSX105.ger.corp.intel.com> <54E9C2C0.3090108@bisdn.de> <54F49E9D.6070201@bisdn.de> <20150303093355.GA7300@bricha3-MOBL3> <54F5A6CF.2090203@bisdn.de> <54F5ABCA.4010707@redhat.com> <54F5AF73.1060109@bisdn.de> <20150303130301.GA11084@bricha3-MOBL3> In-Reply-To: <20150303130301.GA11084@bricha3-MOBL3> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH] mk: add support for gdb debug info generation X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 03 Mar 2015 13:31:53 -0000 > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson > Sent: Tuesday, March 03, 2015 1:03 PM > To: Marc Sune > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] mk: add support for gdb debug info genera= tion >=20 > On Tue, Mar 03, 2015 at 01:56:19PM +0100, Marc Sune wrote: > > > > On 03/03/15 13:40, Panu Matilainen wrote: > > >On 03/03/2015 02:19 PM, Marc Sune wrote: > > >> > > >>On 03/03/15 10:33, Bruce Richardson wrote: > > >>>On Mon, Mar 02, 2015 at 06:32:13PM +0100, Marc Sune wrote: > > >>>>On 22/02/15 12:51, Marc Sune wrote: > > >>>>>I don't like the proposed patch, but I am recovering this old thre= ad > > >>>>>because I agree on the problem statement. > > >>>>> > > >>>>>On 04/04/14 11:57, Ananyev, Konstantin wrote: > > >>>>>>Hi Cyril, > > >>>>>>We already do have 'EXTRA_CFLAGS' and 'EXTRA_LDFLAGS' that you > > >>>>>>can use > > >>>>>>to enable debug, or any other compiler/linker options you need. > > >>>>>>Wonder, why that is not enough? > > >>>>>EXTRA_FLAGS var affects all the DPDK libraries. I was wondering wh= y > > >>>>>setting individually: > > >>>>> > > >>>>>diff --git a/config/common_linuxapp b/config/common_linuxapp > > >>>>>index 2f9643b..04adc0d 100644 > > >>>>>--- a/config/common_linuxapp > > >>>>>+++ b/config/common_linuxapp > > >>>>>@@ -127,7 +127,7 @@ CONFIG_RTE_LIBRTE_KVARGS=3Dy > > >>>>># Compile generic ethernet library > > >>>>># > > >>>>>CONFIG_RTE_LIBRTE_ETHER=3Dy > > >>>>>-CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=3Dn > > >>>>>+CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=3Dy > > >>>>> > > >>>>> > > >>>>>to put an example, does not set -g and -O0 in that particular modu= le > > >>>>>only. > > >>>>>No one would ever use something compiled in DEBUG in production > > >>>>>anyway. > > >>>>> > > >>>>>I always end up modifying manually Makefiles in the lib library th= at > > >>>>>I am > > >>>>>interested in having insides, overriding CFLAGS=3D-O3, which is no= t > > >>>>>that > > >>>>>nice. > > >>>>I would like some feedback on this idea. If the community sees > > >>>>benefit, I > > >>>>will work on a patch for this. > > >>>> > > >>>>Marc > > >>>> > > >>>So, your proposal is to patch things so that any time one sets DEBUG= =3Dy > > >>>in the > > >>>build-time config for a library, we change the '-O3' to '-O0' and se= t > > >>>-g also. > > >>>Correct? > > >> > > >>I am not sure what you mean by 'patch things'. I would simply enable = the > > >>build system to override the default compilation flags (now DPDK-wide= , > > >>or specifically librte_ wide) when _DEBUG=3Dy for a library, changing > > >>compilation flags from -O3 to -O0 -g and possibly also -fno-inline. I > > >>have to check if -O0 already implicitly means -fno-inline (even for > > >>__attribute__((always_inline)) ). > > >> > > >>I did a quick test. I chose KNI because it didn't have a DEBUG flag f= or > > >>the user-space library. For other libraries, the existing _DEBUG sett= ing > > >>would be enough: > > >> > > >>marc@dpdk:~/dpdk$ git diff HEAD > > >>diff --git a/config/common_linuxapp b/config/common_linuxapp > > >>index 97f1c9e..8a3cef8 100644 > > >>--- a/config/common_linuxapp > > >>+++ b/config/common_linuxapp > > >>@@ -405,6 +405,7 @@ CONFIG_RTE_LIBRTE_PIPELINE=3Dy > > >> # > > >> CONFIG_RTE_LIBRTE_KNI=3Dy > > >> CONFIG_RTE_KNI_PREEMPT_DEFAULT=3Dy > > >>+CONFIG_RTE_LIBRTE_KNI_DEBUG=3Dy > > >> CONFIG_RTE_KNI_KO_DEBUG=3Dn > > >> CONFIG_RTE_KNI_VHOST=3Dn > > >> CONFIG_RTE_KNI_VHOST_MAX_CACHE_SIZE=3D1024 > > >>diff --git a/lib/librte_kni/Makefile b/lib/librte_kni/Makefile > > >>index 7107832..895f64e 100644 > > >>--- a/lib/librte_kni/Makefile > > >>+++ b/lib/librte_kni/Makefile > > >>@@ -34,7 +34,7 @@ include $(RTE_SDK)/mk/rte.vars.mk > > >> # library name > > >> LIB =3D librte_kni.a > > >> > > >>-CFLAGS +=3D $(WERROR_FLAGS) -I$(SRCDIR) -O3 -fno-strict-aliasing > > >>+CFLAGS +=3D $(WERROR_FLAGS) -I$(SRCDIR) $(CONFIG_RTE_LIBRTE_KNI_CFLA= GS) > > >> > > >> EXPORT_MAP :=3D rte_kni_version.map > > >> > > >>diff --git a/mk/rte.app.mk b/mk/rte.app.mk > > >>index 63a41e2..eee477d 100644 > > >>--- a/mk/rte.app.mk > > >>+++ b/mk/rte.app.mk > > >>@@ -78,6 +78,13 @@ endif > > >> ifeq ($(CONFIG_RTE_LIBRTE_KNI),y) > > >> ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) > > >> LDLIBS +=3D -lrte_kni > > >>+ > > >>+ifeq ($(CONFIG_RTE_LIBRTE_KNI_DEBUG),y) > > >>+CONFIG_RTE_LIBRTE_KNI_CFLAGS =3D -O0 -g -fno-strict-aliasing -fno-in= line > > >>+else > > >>+CONFIG_RTE_LIBRTE_KNI_CFLAGS =3D -O3 -fno-strict-aliasing > > >>+endif > > >>+ > > >> endif > > >> endif > > >> > > >> > > >>Thoughts? > > > > > >My 5c is that if anything, DPDK needs *less* places that muck around w= ith > > >compiler flags, not more. If you something like this for all the libra= ries > > >in DPDK the number doesn't just increase a bit, it explodes. > > > > If you check the part below this one in my original email, that you str= ipped > > out (without notice), the suggestion was also to add a global _DEBUG > > parameter for the entire DPDK set of libraries, to change all the CFLAG= S at > > once (not in the attached PATCH). > > > > > > > >I dont see that much point in this thing, but I'd approach it by defin= ing > > >the debug flags someplace central, say DEBUG_FLAGS, and append that to= the > > >common cflags when *_DEBUG config is enabled. At least with gcc the la= st > > >option wins so if you just append -O0 when debugging then that's what > > >wins, the earlier -O3 does not matter. > > > > The original problem is the one you expose; libraries hardcode the CFLA= GS, > > ignoring user-flags. There is no way to change this unless you change t= he > > Makefiles directly. > > > > But right now, each library does hardcode its *own* flags (check Makefi= les > > for the libraries), so there is already not a unified approach here. I = see > > for instance KNI having -fno-strict-aliasing while other libraries don'= t. > > > > Having said that, there are moments, specially with -O3, in which to be= able > > to reproduce a bug, you need to compile certain parts of code with -O3 = and > > the rest with -O0 -g (the ones to be debugged). The approach proposed (= both > > a global *and* a lib specific) allows that. > > > > Marc > > >=20 > I believe that the global option of overriding the CFLAGS is already suff= iciently > covered - including being documented in programmers guide - by EXTRA_CFLA= GS. The > ability to turn off optimization support for a single library is not cove= red > anywhere, and that suggestion seems reasonable to me. For each library, w= e can > just append '-O0 -g' to the CFLAGS in that libraries makefile if the debu= g option > is set. I don't see that as significantly complicating things [though I w= ouldn't > make any changes to the rte.app.mk to allow this, just have it per lib in= the > lib's makefile] >=20 Well, but then people would say 'why only -g -O0' for _DEBUG config option? And would ask for ability to add/change other compile options to add/change= for particular library. If we need that, probably we can introduce RTE_LIBRTE__EXTRA_CFLAG= S config macros for each library? So people can set them in their config files to whatever they like?=20 Konstantin