From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 03B3768CA for ; Mon, 3 Oct 2016 18:27:34 +0200 (CEST) Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga105.fm.intel.com with ESMTP; 03 Oct 2016 09:27:33 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,438,1473145200"; d="scan'208";a="15706180" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga006.fm.intel.com with ESMTP; 03 Oct 2016 09:27:33 -0700 Received: from fmsmsx157.amr.corp.intel.com (10.18.116.73) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 3 Oct 2016 09:27:33 -0700 Received: from fmsmsx114.amr.corp.intel.com ([169.254.6.222]) by FMSMSX157.amr.corp.intel.com ([169.254.14.96]) with mapi id 14.03.0248.002; Mon, 3 Oct 2016 09:27:33 -0700 From: "Wiles, Keith" To: Olivier Matz CC: Thomas Monjalon , "dev@dpdk.org" , "david.marchand@6wind.com" Thread-Topic: [dpdk-dev] [PATCH] log: do not drop debug logs at compile time Thread-Index: AQHSD+4sqGLXH6F1Bkisr12qzYWg4qCSUNQAgABowoCABKooAIAAByuAgAACsgCAAA32AA== Date: Mon, 3 Oct 2016 16:27:32 +0000 Message-ID: <036D8827-DFD7-421D-8133-3011D42CEAB4@intel.com> References: <1474011832-29987-1-git-send-email-olivier.matz@6wind.com> <2969629.OOuc7vtRSA@xps13> <104cb490-b39a-58e6-2031-d99e023474f2@6wind.com> In-Reply-To: <104cb490-b39a-58e6-2031-d99e023474f2@6wind.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.255.230.84] Content-Type: text/plain; charset="us-ascii" Content-ID: <74901A205ACDAF46B6A2B8750B75DA32@intel.com> Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] log: do not drop debug logs at compile time 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: Mon, 03 Oct 2016 16:27:35 -0000 Regards, Keith > On Oct 3, 2016, at 10:37 AM, Olivier Matz wrote: >=20 >=20 >=20 > On 10/03/2016 05:27 PM, Wiles, Keith wrote: >>=20 >> Regards, >> Keith >>=20 >>> On Oct 3, 2016, at 10:02 AM, Olivier Matz wrot= e: >>>=20 >>> Hi Keith, >>>=20 >>> On 09/30/2016 05:48 PM, Wiles, Keith wrote: >>>>> On Sep 30, 2016, at 4:33 AM, Thomas Monjalon wrote: >>>>>=20 >>>>> 2016-09-16 09:43, Olivier Matz: >>>>>> Today, all logs whose level is lower than INFO are dropped at >>>>>> compile-time. This prevents from enabling debug logs at runtime usin= g >>>>>> --log-level=3D8. >>>>>>=20 >>>>>> The rationale was to remove debug logs from the data path at >>>>>> compile-time, avoiding a test at run-time. >>>>>>=20 >>>>>> This patch changes the behavior of RTE_LOG() to avoid the compile-ti= me >>>>>> optimization, and introduces the RTE_LOG_DP() macro that has the sam= e >>>>>> behavior than the previous RTE_LOG(), for the rare cases where debug >>>>>> logs are in the data path. >>>>>>=20 >>>>>> So it is now possible to enable debug logs at run-time by just >>>>>> specifying --log-level=3D8. Some drivers still have special compile-= time >>>>>> options to enable more debug log. Maintainers may consider to >>>>>> remove/reduce them. >>>>>>=20 >>>>>> Signed-off-by: Olivier Matz >>>>>=20 >>>>> I think it is a good change. >>>>> However I'm not sure we should take it for 16.11 as it was sent late = and >>>>> there is no review comment. >>>>> It is neither really a fix nor really a feature. >>>>> If there are some +1, and no opinions against, it will go in 16.11. >>>>> Note that some drivers would need some changes to fully benefit of >>>>> debug logs enabled at run-time. >>>>=20 >>>> Would this be easier to add a new LOG level instead say DEBUG_DATAPATH= and then change the RTE_LOG to exclude the new log level? >>>>=20 >>>>=20 >>>=20 >>> The log levels are quite standard, I don't feel it would be very clear >>> to have a new level for that. It would also prevent to have different >>> log level inside data path. >>=20 >> I am not following you here. Having one more log level for DEBUG in the = data path is not a big change and you can still have any other log level in= the data or anyplace else for that matter. >=20 > Adding a new log level is not a big change, you are right. > But to me it looks confusing to have DEBUG, INFO, ..., WARNING, ERROR, > plus a DEBUG_DATAPATH. For instance, how do you compare levels? Or if > your log stream forwards logs to syslog, you cannot do a 1:1 mapping > with standard syslog levels. Doing 1:1 mapping is not a big problem unless you are trying to compare to = old logs, which maybe the case the first time and after that it should not = be a problem. >=20 > What makes you feel it's easier to add a log level instead of adding a > new RTE_LOG_DP() function? It seems to me the log levels are for displaying logs at different levels a= dding a new macro to not log is just a hack because we do not have a log le= vel for data path. This is why I would like to see a log level added and no= t a new macro. It also appears the new RTE_LOG() will always be in the code as you moved t= he test to the RTE_LOG_DP() macro. This would mean all RTE_LOG() in the cod= e will always call rte_log(), correct? If using a new DEBUG_DP (maybe DATAPATH is a better log level name) level w= e can use the same macro as before and modify the level only. This way we c= an remove via the compiler any log that is below the default RTE_LOG_LEVEL.= I see keeping the rte_log() could be a performance problem or code blot wh= en you really want to remove them all. The DATAPATH log level would be above (smaller number) then DEBUG in the en= um list. To remove all debug logs just set the RTE_LOG_LEVEL to RTE_LOG_DAT= APATH. >=20 >=20 > Regards, > Olivier