From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 7589CA00C2 for ; Thu, 10 Feb 2022 10:49:31 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1768740041; Thu, 10 Feb 2022 10:49:31 +0100 (CET) Received: from smtpbgeu2.qq.com (smtpbgeu2.qq.com [18.194.254.142]) by mails.dpdk.org (Postfix) with ESMTP id CBC0140041 for ; Thu, 10 Feb 2022 10:49:29 +0100 (CET) X-QQ-mid: bizesmtp6t1644486565t1n36qj7z Received: from jiawenwu (unknown [183.129.236.74]) by bizesmtp.qq.com (ESMTP) with id ; Thu, 10 Feb 2022 17:49:24 +0800 (CST) X-QQ-SSF: 01400000002000F0L000000A0000000 X-QQ-FEAT: ZHWZeLXy+8eRrX8LrXOxB3js5V81qgdfyU74izw2WBqNtm8gmBG3yn+b0jV89 PWIP1lxNCNN2b298saVhgU8/9dzhxcElqrXo+Yq4GYsDukyfIpK3hqwhHNVf/5kYH8QbDGA dVCsOOvG4XhI58W1VLfIUn43oaOtMgE4GqxK3yhMn3mJ2ASp2uuSsxMcxoTeMLDf4SfHdUB Vxu0XEtH0KBzLURbf0Tmv77IpmBnrI8vfNpKGnkAfVgnX/mW3ZlNFXbKn2T70KYUUpl7qDg piGkbqDN1hbtD4qLOivs9AVq4OspJ4AWU38Rh7DRZwAkmuf6g45bZRJ0MpkwHss74lCourY nruu3A9b0uTdFyElQhk9QIqb10c49qI7AaxXja5 X-QQ-GoodBg: 2 From: "Jiawen Wu" To: "'Ferruh Yigit'" , Cc: References: <20220209104213.602728-1-jiawenwu@trustnetic.com> <20220209104213.602728-9-jiawenwu@trustnetic.com> <004201d81e54$c0401160$40c03420$@trustnetic.com> <09f2fe07-b15b-9b05-b57c-54eaef14fc94@intel.com> In-Reply-To: <09f2fe07-b15b-9b05-b57c-54eaef14fc94@intel.com> Subject: RE: [PATCH v2 08/12] net/ngbe: fix debug log Date: Thu, 10 Feb 2022 17:49:23 +0800 Message-ID: <004501d81e63$7cb39890$761ac9b0$@trustnetic.com>+8AC30EE9201149CA MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Mailer: Microsoft Outlook 16.0 Content-Language: zh-cn Thread-Index: AQGgZ+TcfTHBt4TpTBeLIOldsX01eQI3FN5CAsnA/CEBUHWKVgJwv+xrrLXlr9A= X-QQ-SENDSIZE: 520 Feedback-ID: bizesmtp:trustnetic.com:qybgforeign:qybgforeign6 X-QQ-Bgrelay: 1 X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org On February 10, 2022 5:03 PM, Ferruh Yigit wrote: > On 2/10/2022 8:03 AM, Jiawen Wu wrote: > > On February 10, 2022 3:07 AM, Ferruh Yigit wrote: > >> On 2/9/2022 10:42 AM, Jiawen Wu wrote: > >>> Remove 'DEBUGFUNC' due to too many invalid debug log prints. And = fix > >>> that double line was added by using 'DEBUGOUT'. > >>> > >>> Fixes: cc934df178ab ("net/ngbe: add log and error types") > >>> Cc: stable@dpdk.org > >>> > >>> Signed-off-by: Jiawen Wu > >>> --- > >>> drivers/net/ngbe/ngbe_logs.h | 11 +++++++---- > >>> 1 file changed, 7 insertions(+), 4 deletions(-) > >>> > >>> diff --git a/drivers/net/ngbe/ngbe_logs.h > >>> b/drivers/net/ngbe/ngbe_logs.h index fd306419e6..b7d78fb400 100644 > >>> --- a/drivers/net/ngbe/ngbe_logs.h > >>> +++ b/drivers/net/ngbe/ngbe_logs.h > >>> @@ -15,9 +15,12 @@ extern int ngbe_logtype_init; > >>> "%s(): " fmt "\n", __func__, ##args) > >>> > >>> extern int ngbe_logtype_driver; > >>> -#define PMD_DRV_LOG(level, fmt, args...) \ > >>> +#define PMD_TLOG_DRIVER(level, fmt, args...) \ > >>> rte_log(RTE_LOG_ ## level, ngbe_logtype_driver, \ > >>> - "%s(): " fmt "\n", __func__, ##args) > >>> + "%s(): " fmt, __func__, ##args) > >>> + > >>> +#define PMD_DRV_LOG(level, fmt, args...) \ > >>> + PMD_TLOG_DRIVER(level, fmt "\n", ## args) > >>> > >> > >> Both 'DEBUGOUT' and 'PMD_DRV_LOG(DEBUG, ..' are in use for same > >> thing, but one appends '\n' other doesn't, this is very error prune > >> and there are already wrong usage in the driver. > >> I think no need to add complexity for something as simple as this, > >> what do you think to unify the DEBUG level macros, at least unify = the line > ending behavior? > >> > >> > >>> #ifdef RTE_ETHDEV_DEBUG_RX > >>> extern int ngbe_logtype_rx; > >>> @@ -37,10 +40,10 @@ extern int ngbe_logtype_tx; > >>> #define PMD_TX_LOG(level, fmt, args...) do { } while (0) > >>> #endif > >>> > >>> -#define TLOG_DEBUG(fmt, args...) PMD_DRV_LOG(DEBUG, fmt, > ##args) > >>> +#define TLOG_DEBUG(fmt, args...) PMD_TLOG_DRIVER(DEBUG, fmt, > >> ##args) > >>> > >>> #define DEBUGOUT(fmt, args...) TLOG_DEBUG(fmt, ##args) > >>> #define PMD_INIT_FUNC_TRACE() TLOG_DEBUG(" >>") > >> > >> What is difference between 'PMD_INIT_FUNC_TRACE' and 'DEBUGFUNC'? > >> As far as I can see they are for same reason, and both macros are > >> used. If they are for same reason can you please unify the usage? > >> > >>> -#define DEBUGFUNC(fmt) TLOG_DEBUG(fmt) > >>> +#define DEBUGFUNC(fmt) do { } while (0) > >> > >> If 'DEBUGFUNC' can be removed, why not removing from the code, > >> instead of above define? This is your driver, you have full control = on it. > > > > Okay. I just realize that this is going to be a big change, so I = want to keep it to > a minimum. > > Anyway, 'DEBUGFUNC' should be removed, because 'DEBUGOUT' already > contains the function name. > > >=20 > If it will be big, we can move the fix out of this set, to not block = set, and send a > log fix later as a separate patch, what do you think? I think it's actually better.