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 BA177A00BE; Fri, 11 Feb 2022 03:02:24 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id A240341141; Fri, 11 Feb 2022 03:02:24 +0100 (CET) Received: from smtpbg501.qq.com (smtpbg501.qq.com [203.205.250.101]) by mails.dpdk.org (Postfix) with ESMTP id 015804115A for ; Fri, 11 Feb 2022 03:02:21 +0100 (CET) X-QQ-mid: bizesmtp32t1644544928tkqpu5fh Received: from jiawenwu (unknown [183.129.236.74]) by bizesmtp.qq.com (ESMTP) with id ; Fri, 11 Feb 2022 10:02:07 +0800 (CST) X-QQ-SSF: 01400000000000F0M000B00A0000000 X-QQ-FEAT: xyySF+ktKzf+ZQ64HyzG7BnCB5Rzf8T9THSmVlfXqyBj5li3NBC9KriPOXWi9 UASicEG9ZgQYbuyOBHcd9H5QFHrPMU7aEtLiFrx81R/MdGtCefI1vaU0wAs5Rwu5otNttJw 1Cve/tdnoZRIlpgGTLk+61YczpjYFR8OfZQzGzJYO+CEWGL5SUJCcXxQeErSCyHMBF3Q/D+ oXznoS/GdsDZIBsj06wFwqiSZmpbsH8qRfaqC591Ju1ytCn9OLh2hu0IN8JhpINOCUG0RQc 8MS8zOwCXPvT19pYQ+hunjTUhA5iNosCwKi8nhM3hlbeiIHnOy7GHp0KfWXho5CT8iHZ14f xOWTBZtoxB/2xdFUqbeTdtRXDtPxQ== 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> <004501d81e63$7cb39890$761ac9b0$@trustnetic.com> <56bcd034-1e9d-c172-b449-3ee71d2a2835@intel.com> In-Reply-To: <56bcd034-1e9d-c172-b449-3ee71d2a2835@intel.com> Subject: RE: [PATCH v2 08/12] net/ngbe: fix debug log Date: Fri, 11 Feb 2022 10:02:07 +0800 Message-ID: <002601d81eeb$60403c80$20c0b580$@trustnetic.com>+EF12AD9455C34E9F 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+xrAaxFIUUCBIXtWqyZbWtQ X-QQ-SENDSIZE: 520 Feedback-ID: bizesmtp:trustnetic.com:qybgforeign:qybgforeign1 X-QQ-Bgrelay: 1 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On February 10, 2022 6:16 PM, Ferruh Yigit wrote: > On 2/10/2022 9:49 AM, Jiawen Wu wrote: > > 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. > >>> > >> > >> 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. > > >=20 > As long as you commit to send log fix after -rc1, I am OK. >=20 > Are you planning to send a new version of this set, or will it work if = I just drop > the patch 8/12 & 10/12 of this set and continue with this version? It works, you can continue with this version. Thanks Ferruh.