diff mbox series

[v2,2/5] net/mlx4: use mlx4 debug flag instead of NDEBUG

Message ID 1579803629-152938-3-git-send-email-akozyrev@mellanox.com (mailing list archive)
State Superseded, archived
Delegated to: Raslan Darawsheh
Headers
Series net/mlx: assert cleanup in mlx drivers |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Alexander Kozyrev Jan. 23, 2020, 6:20 p.m. UTC 
  Define a new MLX4_DEBUG compilation flag to get rid of dependency
on the NDEBUG definition. This is a preparation step to switch
from standard assert clauses to DPDK RTE_ASSERT ones in MLX4 driver.

Signed-off-by: Alexander Kozyrev <akozyrev@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
 drivers/net/mlx4/Makefile     |  4 ++--
 drivers/net/mlx4/meson.build  |  4 ++--
 drivers/net/mlx4/mlx4.c       |  4 ++--
 drivers/net/mlx4/mlx4_mr.c    |  8 ++++----
 drivers/net/mlx4/mlx4_rxtx.c  | 10 +++++-----
 drivers/net/mlx4/mlx4_utils.h |  8 ++++----
 6 files changed, 19 insertions(+), 19 deletions(-)

Comments

Ferruh Yigit Jan. 24, 2020, 4:43 p.m. UTC | #1 
On 1/23/2020 6:20 PM, Alexander Kozyrev wrote:
> Define a new MLX4_DEBUG compilation flag to get rid of dependency
> on the NDEBUG definition. This is a preparation step to switch
> from standard assert clauses to DPDK RTE_ASSERT ones in MLX4 driver.
> 
> Signed-off-by: Alexander Kozyrev <akozyrev@mellanox.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> ---
>  drivers/net/mlx4/Makefile     |  4 ++--
>  drivers/net/mlx4/meson.build  |  4 ++--
>  drivers/net/mlx4/mlx4.c       |  4 ++--
>  drivers/net/mlx4/mlx4_mr.c    |  8 ++++----
>  drivers/net/mlx4/mlx4_rxtx.c  | 10 +++++-----
>  drivers/net/mlx4/mlx4_utils.h |  8 ++++----
>  6 files changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/mlx4/Makefile b/drivers/net/mlx4/Makefile
> index 329569d..043e72f 100644
> --- a/drivers/net/mlx4/Makefile
> +++ b/drivers/net/mlx4/Makefile
> @@ -65,13 +65,13 @@ endif
>  
>  # User-defined CFLAGS.
>  ifeq ($(CONFIG_RTE_LIBRTE_MLX4_DEBUG),y)
> -CFLAGS += -pedantic -UNDEBUG
> +CFLAGS += -pedantic -DMLX4_DEBUG

Can't use 'RTE_LIBRTE_MLX4_DEBUG' directly in the .c files, instead of interim
'MLX4_DEBUG', many other config options used that way.

>  ifneq ($(CONFIG_RTE_TOOLCHAIN_ICC),y)
>  CFLAGS += -DPEDANTIC
>  endif
>  AUTO_CONFIG_CFLAGS += -Wno-pedantic
>  else
> -CFLAGS += -DNDEBUG -UPEDANTIC
> +CFLAGS += -UMLX4_DEBUG -UPEDANTIC
>  endif
>  
>  include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/drivers/net/mlx4/meson.build b/drivers/net/mlx4/meson.build
> index 9eb4988..a15a301 100644
> --- a/drivers/net/mlx4/meson.build
> +++ b/drivers/net/mlx4/meson.build
> @@ -67,9 +67,9 @@ if build
>  		endif
>  	endforeach
>  	if get_option('buildtype').contains('debug')
> -		cflags += [ '-pedantic', '-UNDEBUG', '-DPEDANTIC' ]
> +		cflags += [ '-pedantic', '-DMLX4_DEBUG', '-DPEDANTIC' ]
>  	else
> -		cflags += [ '-DNDEBUG', '-UPEDANTIC' ]
> +		cflags += [ '-UMLX4_DEBUG', '-UPEDANTIC' ]

Right now there is no way in meson to enable/disable compile time (datapath)
debug options in module granularity, it would be good to have them.
Slava Ovsiienko Jan. 24, 2020, 4:50 p.m. UTC | #2 
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Friday, January 24, 2020 18:43
> To: Alexander Kozyrev <akozyrev@mellanox.com>; dev@dpdk.org
> Cc: Raslan Darawsheh <rasland@mellanox.com>; Matan Azrad
> <matan@mellanox.com>; Slava Ovsiienko <viacheslavo@mellanox.com>;
> Thomas Monjalon <thomas@monjalon.net>; Bruce Richardson
> <bruce.richardson@intel.com>
> Subject: Re: [PATCH v2 2/5] net/mlx4: use mlx4 debug flag instead of NDEBUG
> 
> On 1/23/2020 6:20 PM, Alexander Kozyrev wrote:
> > Define a new MLX4_DEBUG compilation flag to get rid of dependency on
> > the NDEBUG definition. This is a preparation step to switch from
> > standard assert clauses to DPDK RTE_ASSERT ones in MLX4 driver.
> >
> > Signed-off-by: Alexander Kozyrev <akozyrev@mellanox.com>
> > Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> > ---
> >  drivers/net/mlx4/Makefile     |  4 ++--
> >  drivers/net/mlx4/meson.build  |  4 ++--
> >  drivers/net/mlx4/mlx4.c       |  4 ++--
> >  drivers/net/mlx4/mlx4_mr.c    |  8 ++++----
> >  drivers/net/mlx4/mlx4_rxtx.c  | 10 +++++-----
> > drivers/net/mlx4/mlx4_utils.h |  8 ++++----
> >  6 files changed, 19 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/net/mlx4/Makefile b/drivers/net/mlx4/Makefile
> > index 329569d..043e72f 100644
> > --- a/drivers/net/mlx4/Makefile
> > +++ b/drivers/net/mlx4/Makefile
> > @@ -65,13 +65,13 @@ endif
> >
> >  # User-defined CFLAGS.
> >  ifeq ($(CONFIG_RTE_LIBRTE_MLX4_DEBUG),y)
> > -CFLAGS += -pedantic -UNDEBUG
> > +CFLAGS += -pedantic -DMLX4_DEBUG
> 
> Can't use 'RTE_LIBRTE_MLX4_DEBUG' directly in the .c files, instead of interim
> 'MLX4_DEBUG', many other config options used that way.
> 
> >  ifneq ($(CONFIG_RTE_TOOLCHAIN_ICC),y)  CFLAGS += -DPEDANTIC  endif
> > AUTO_CONFIG_CFLAGS += -Wno-pedantic  else -CFLAGS += -DNDEBUG
> > -UPEDANTIC
> > +CFLAGS += -UMLX4_DEBUG -UPEDANTIC
> >  endif
> >
> >  include $(RTE_SDK)/mk/rte.lib.mk
> > diff --git a/drivers/net/mlx4/meson.build
> > b/drivers/net/mlx4/meson.build index 9eb4988..a15a301 100644
> > --- a/drivers/net/mlx4/meson.build
> > +++ b/drivers/net/mlx4/meson.build
> > @@ -67,9 +67,9 @@ if build
> >  		endif
> >  	endforeach
> >  	if get_option('buildtype').contains('debug')
> > -		cflags += [ '-pedantic', '-UNDEBUG', '-DPEDANTIC' ]
> > +		cflags += [ '-pedantic', '-DMLX4_DEBUG', '-DPEDANTIC' ]
> >  	else
> > -		cflags += [ '-DNDEBUG', '-UPEDANTIC' ]
> > +		cflags += [ '-UMLX4_DEBUG', '-UPEDANTIC' ]
> 
> Right now there is no way in meson to enable/disable compile time
> (datapath) debug options in module granularity, it would be good to have
> them.
+1, I think we should not drop the module debug options and
it would be good to support ones with meson.

With best regards, Slava
Bruce Richardson Jan. 24, 2020, 5:02 p.m. UTC | #3 
On Fri, Jan 24, 2020 at 04:50:51PM +0000, Slava Ovsiienko wrote:
> > -----Original Message-----
> > From: Ferruh Yigit <ferruh.yigit@intel.com>
> > Sent: Friday, January 24, 2020 18:43
> > To: Alexander Kozyrev <akozyrev@mellanox.com>; dev@dpdk.org
> > Cc: Raslan Darawsheh <rasland@mellanox.com>; Matan Azrad
> > <matan@mellanox.com>; Slava Ovsiienko <viacheslavo@mellanox.com>;
> > Thomas Monjalon <thomas@monjalon.net>; Bruce Richardson
> > <bruce.richardson@intel.com>
> > Subject: Re: [PATCH v2 2/5] net/mlx4: use mlx4 debug flag instead of NDEBUG
> > 
> > On 1/23/2020 6:20 PM, Alexander Kozyrev wrote:
> > > Define a new MLX4_DEBUG compilation flag to get rid of dependency on
> > > the NDEBUG definition. This is a preparation step to switch from
> > > standard assert clauses to DPDK RTE_ASSERT ones in MLX4 driver.
> > >
> > > Signed-off-by: Alexander Kozyrev <akozyrev@mellanox.com>
> > > Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> > > ---
> > >  drivers/net/mlx4/Makefile     |  4 ++--
> > >  drivers/net/mlx4/meson.build  |  4 ++--
> > >  drivers/net/mlx4/mlx4.c       |  4 ++--
> > >  drivers/net/mlx4/mlx4_mr.c    |  8 ++++----
> > >  drivers/net/mlx4/mlx4_rxtx.c  | 10 +++++-----
> > > drivers/net/mlx4/mlx4_utils.h |  8 ++++----
> > >  6 files changed, 19 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/drivers/net/mlx4/Makefile b/drivers/net/mlx4/Makefile
> > > index 329569d..043e72f 100644
> > > --- a/drivers/net/mlx4/Makefile
> > > +++ b/drivers/net/mlx4/Makefile
> > > @@ -65,13 +65,13 @@ endif
> > >
> > >  # User-defined CFLAGS.
> > >  ifeq ($(CONFIG_RTE_LIBRTE_MLX4_DEBUG),y)
> > > -CFLAGS += -pedantic -UNDEBUG
> > > +CFLAGS += -pedantic -DMLX4_DEBUG
> > 
> > Can't use 'RTE_LIBRTE_MLX4_DEBUG' directly in the .c files, instead of interim
> > 'MLX4_DEBUG', many other config options used that way.
> > 
> > >  ifneq ($(CONFIG_RTE_TOOLCHAIN_ICC),y)  CFLAGS += -DPEDANTIC  endif
> > > AUTO_CONFIG_CFLAGS += -Wno-pedantic  else -CFLAGS += -DNDEBUG
> > > -UPEDANTIC
> > > +CFLAGS += -UMLX4_DEBUG -UPEDANTIC
> > >  endif
> > >
> > >  include $(RTE_SDK)/mk/rte.lib.mk
> > > diff --git a/drivers/net/mlx4/meson.build
> > > b/drivers/net/mlx4/meson.build index 9eb4988..a15a301 100644
> > > --- a/drivers/net/mlx4/meson.build
> > > +++ b/drivers/net/mlx4/meson.build
> > > @@ -67,9 +67,9 @@ if build
> > >  		endif
> > >  	endforeach
> > >  	if get_option('buildtype').contains('debug')
> > > -		cflags += [ '-pedantic', '-UNDEBUG', '-DPEDANTIC' ]
> > > +		cflags += [ '-pedantic', '-DMLX4_DEBUG', '-DPEDANTIC' ]
> > >  	else
> > > -		cflags += [ '-DNDEBUG', '-UPEDANTIC' ]
> > > +		cflags += [ '-UMLX4_DEBUG', '-UPEDANTIC' ]
> > 
> > Right now there is no way in meson to enable/disable compile time
> > (datapath) debug options in module granularity, it would be good to have
> > them.
> +1, I think we should not drop the module debug options and
> it would be good to support ones with meson.
> 
Discussing with Ferruh offline, one option might be to add a generic
--drivers-debug flag [taking the same parameters as the current
disable-drivers option], and defines a debug flag (or flags) for the
specified driver. To keep things simple for drivers, we could just blindly
define for each requested driver the macros similar to that used in make
such as ..._DEBUG_RX. _DEBUG_TX.

Regards,
/Bruce
diff mbox series

Patch

diff --git a/drivers/net/mlx4/Makefile b/drivers/net/mlx4/Makefile
index 329569d..043e72f 100644
--- a/drivers/net/mlx4/Makefile
+++ b/drivers/net/mlx4/Makefile
@@ -65,13 +65,13 @@  endif
 
 # User-defined CFLAGS.
 ifeq ($(CONFIG_RTE_LIBRTE_MLX4_DEBUG),y)
-CFLAGS += -pedantic -UNDEBUG
+CFLAGS += -pedantic -DMLX4_DEBUG
 ifneq ($(CONFIG_RTE_TOOLCHAIN_ICC),y)
 CFLAGS += -DPEDANTIC
 endif
 AUTO_CONFIG_CFLAGS += -Wno-pedantic
 else
-CFLAGS += -DNDEBUG -UPEDANTIC
+CFLAGS += -UMLX4_DEBUG -UPEDANTIC
 endif
 
 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/net/mlx4/meson.build b/drivers/net/mlx4/meson.build
index 9eb4988..a15a301 100644
--- a/drivers/net/mlx4/meson.build
+++ b/drivers/net/mlx4/meson.build
@@ -67,9 +67,9 @@  if build
 		endif
 	endforeach
 	if get_option('buildtype').contains('debug')
-		cflags += [ '-pedantic', '-UNDEBUG', '-DPEDANTIC' ]
+		cflags += [ '-pedantic', '-DMLX4_DEBUG', '-DPEDANTIC' ]
 	else
-		cflags += [ '-DNDEBUG', '-UPEDANTIC' ]
+		cflags += [ '-UMLX4_DEBUG', '-UPEDANTIC' ]
 	endif
 	# To maintain the compatibility with the make build system
 	# mlx4_autoconf.h file is still generated.
diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
index ab5e6c6..e37ae23 100644
--- a/drivers/net/mlx4/mlx4.c
+++ b/drivers/net/mlx4/mlx4.c
@@ -305,7 +305,7 @@  struct mlx4_conf {
 		      (void *)dev, strerror(-ret));
 		goto err;
 	}
-#ifndef NDEBUG
+#ifdef MLX4_DEBUG
 	mlx4_mr_dump_dev(dev);
 #endif
 	ret = mlx4_rxq_intr_enable(priv);
@@ -1305,7 +1305,7 @@  struct mlx4_conf {
 		return;
 	assert(mlx4_glue);
 #endif
-#ifndef NDEBUG
+#ifdef MLX4_DEBUG
 	/* Glue structure must not contain any NULL pointers. */
 	{
 		unsigned int i;
diff --git a/drivers/net/mlx4/mlx4_mr.c b/drivers/net/mlx4/mlx4_mr.c
index 470cee0..069a450 100644
--- a/drivers/net/mlx4/mlx4_mr.c
+++ b/drivers/net/mlx4/mlx4_mr.c
@@ -242,7 +242,7 @@  struct mr_update_mp_data {
 	memset(bt, 0, sizeof(*bt));
 }
 
-#ifndef NDEBUG
+#ifdef MLX4_DEBUG
 /**
  * Dump all the entries in a B-tree
  *
@@ -962,7 +962,7 @@  struct mr_update_mp_data {
 		rte_smp_wmb();
 	}
 	rte_rwlock_write_unlock(&priv->mr.rwlock);
-#ifndef NDEBUG
+#ifdef MLX4_DEBUG
 	if (rebuild)
 		mlx4_mr_dump_dev(dev);
 #endif
@@ -1380,7 +1380,7 @@  struct mr_update_mp_data {
 	return data.ret;
 }
 
-#ifndef NDEBUG
+#ifdef MLX4_DEBUG
 /**
  * Dump all the created MRs and the global cache entries.
  *
@@ -1440,7 +1440,7 @@  struct mr_update_mp_data {
 	rte_rwlock_write_lock(&mlx4_shared_data->mem_event_rwlock);
 	LIST_REMOVE(priv, mem_event_cb);
 	rte_rwlock_write_unlock(&mlx4_shared_data->mem_event_rwlock);
-#ifndef NDEBUG
+#ifdef MLX4_DEBUG
 	mlx4_mr_dump_dev(dev);
 #endif
 	rte_rwlock_write_lock(&priv->mr.rwlock);
diff --git a/drivers/net/mlx4/mlx4_rxtx.c b/drivers/net/mlx4/mlx4_rxtx.c
index 4dc0adb..a8e880e 100644
--- a/drivers/net/mlx4/mlx4_rxtx.c
+++ b/drivers/net/mlx4/mlx4_rxtx.c
@@ -321,7 +321,7 @@  struct tso_info {
 		if (unlikely(!!(cqe->owner_sr_opcode & MLX4_CQE_OWNER_MASK) ^
 		    !!(cons_index & cq->cqe_cnt)))
 			break;
-#ifndef NDEBUG
+#ifdef MLX4_DEBUG
 		/*
 		 * Make sure we read the CQE after we read the ownership bit.
 		 */
@@ -336,7 +336,7 @@  struct tso_info {
 			      cqe_err->syndrome);
 			break;
 		}
-#endif /* NDEBUG */
+#endif /* MLX4_DEBUG */
 		cons_index++;
 	} while (1);
 	completed = (cons_index - cq->cons_index) * txq->elts_comp_cd_init;
@@ -488,14 +488,14 @@  struct tso_info {
 				((uintptr_t)dseg & (MLX4_TXBB_SIZE - 1))) >>
 			       MLX4_SEG_SHIFT;
 		switch (nb_segs_txbb) {
-#ifndef NDEBUG
+#ifdef MLX4_DEBUG
 		default:
 			/* Should never happen. */
 			rte_panic("%p: Invalid number of SGEs(%d) for a TXBB",
 			(void *)txq, nb_segs_txbb);
 			/* rte_panic never returns. */
 			break;
-#endif /* NDEBUG */
+#endif /* MLX4_DEBUG */
 		case 4:
 			/* Memory region key for this memory pool. */
 			lkey = mlx4_tx_mb2mr(txq, sbuf);
@@ -922,7 +922,7 @@  struct tso_info {
 		if (likely(elt->buf != NULL)) {
 			struct rte_mbuf *tmp = elt->buf;
 
-#ifndef NDEBUG
+#ifdef MLX4_DEBUG
 			/* Poisoning. */
 			memset(&elt->buf, 0x66, sizeof(struct rte_mbuf *));
 #endif
diff --git a/drivers/net/mlx4/mlx4_utils.h b/drivers/net/mlx4/mlx4_utils.h
index 5718b9c..7f9a826 100644
--- a/drivers/net/mlx4/mlx4_utils.h
+++ b/drivers/net/mlx4/mlx4_utils.h
@@ -27,10 +27,10 @@ 
 
 extern int mlx4_logtype;
 
-#ifndef NDEBUG
+#ifdef MLX4_DEBUG
 
 /*
- * When debugging is enabled (NDEBUG not defined), file, line and function
+ * When debugging is enabled (MLX4_DEBUG is defined), file, line and function
  * information replace the driver name (MLX4_DRIVER_NAME) in log messages.
  */
 
@@ -56,7 +56,7 @@ 
 #define DEBUG(...) PMD_DRV_LOG(DEBUG, __VA_ARGS__)
 #define claim_zero(...) assert((__VA_ARGS__) == 0)
 
-#else /* NDEBUG */
+#else /* MLX4_DEBUG */
 
 /*
  * Like assert(), DEBUG() becomes a no-op and claim_zero() does not perform
@@ -71,7 +71,7 @@ 
 #define DEBUG(...) (void)0
 #define claim_zero(...) (__VA_ARGS__)
 
-#endif /* NDEBUG */
+#endif /* MLX4_DEBUG */
 
 #define INFO(...) PMD_DRV_LOG(INFO, __VA_ARGS__)
 #define WARN(...) PMD_DRV_LOG(WARNING, __VA_ARGS__)