From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id A19505A76 for ; Tue, 3 Mar 2015 13:40:45 +0100 (CET) Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t23CeiFD027995 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Tue, 3 Mar 2015 07:40:44 -0500 Received: from localhost.localdomain (vpn1-4-56.ams2.redhat.com [10.36.4.56]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t23Cehh6014884 for ; Tue, 3 Mar 2015 07:40:43 -0500 Message-ID: <54F5ABCA.4010707@redhat.com> Date: Tue, 03 Mar 2015 14:40:42 +0200 From: Panu Matilainen User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: dev@dpdk.org 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> In-Reply-To: <54F5A6CF.2090203@bisdn.de> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 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 12:40:46 -0000 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 thread >>>> 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 why >>>> 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=y >>>> # Compile generic ethernet library >>>> # >>>> CONFIG_RTE_LIBRTE_ETHER=y >>>> -CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=n >>>> +CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=y >>>> >>>> >>>> to put an example, does not set -g and -O0 in that particular module >>>> only. >>>> No one would ever use something compiled in DEBUG in production anyway. >>>> >>>> I always end up modifying manually Makefiles in the lib library that >>>> I am >>>> interested in having insides, overriding CFLAGS=-O3, which is not 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=y >> in the >> build-time config for a library, we change the '-O3' to '-O0' and set >> -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=y 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 for > the user-space library. For other libraries, the existing _DEBUG setting > 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=y > # > CONFIG_RTE_LIBRTE_KNI=y > CONFIG_RTE_KNI_PREEMPT_DEFAULT=y > +CONFIG_RTE_LIBRTE_KNI_DEBUG=y > CONFIG_RTE_KNI_KO_DEBUG=n > CONFIG_RTE_KNI_VHOST=n > CONFIG_RTE_KNI_VHOST_MAX_CACHE_SIZE=1024 > 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 = librte_kni.a > > -CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3 -fno-strict-aliasing > +CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) $(CONFIG_RTE_LIBRTE_KNI_CFLAGS) > > EXPORT_MAP := 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 += -lrte_kni > + > +ifeq ($(CONFIG_RTE_LIBRTE_KNI_DEBUG),y) > +CONFIG_RTE_LIBRTE_KNI_CFLAGS = -O0 -g -fno-strict-aliasing -fno-inline > +else > +CONFIG_RTE_LIBRTE_KNI_CFLAGS = -O3 -fno-strict-aliasing > +endif > + > endif > endif > > > Thoughts? My 5c is that if anything, DPDK needs *less* places that muck around with compiler flags, not more. If you something like this for all the libraries in DPDK the number doesn't just increase a bit, it explodes. I dont see that much point in this thing, but I'd approach it by defining 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 last option wins so if you just append -O0 when debugging then that's what wins, the earlier -O3 does not matter. - Panu - - Panu -