Re: [PATCH RFC rdma-core 1/3] util: Add common code for provider debug

[Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]

> +++ b/buildlib/rdma_functions.cmake
> @@ -6,7 +6,7 @@
>  # Global list of pairs of (SHARED STATIC) libary target names
>  set(RDMA_STATIC_LIBS "" CACHE INTERNAL "Doc" FORCE)
>  
> -set(COMMON_LIBS_PIC ccan_pic)
> +set(COMMON_LIBS_PIC ccan_pic util_pic)
>  set(COMMON_LIBS ccan)

Also change COMMON_LIBS to have util

>  # Install a symlink during 'make install'
> diff --git a/util/CMakeLists.txt b/util/CMakeLists.txt
> index 1cda890..fb36103 100644
> +++ b/util/CMakeLists.txt
> @@ -1,3 +1,11 @@
>  publish_internal_headers(util
>    compiler.h
> +  dbg.h
>    )
> +
> +set(C_FILES
> +  dbg.c
> +  )
> +
> +add_library(util_pic ${C_FILES})

Use these two lines instead:

add_library(util STATIC ${C_FILES})
add_library(util_pic STATIC ${C_FILES})

> +#define VERBS_PROVIDER_DBG_MASK  "VERBS_PROVIDER_DBG_MASK"
> +#define VERBS_PROVIDER_DBG_LEVEL "VERBS_PROVIDER_DBG_LEVEL"
> +#define VERBS_PROVIDER_DBG_FILE  "VERBS_PROVIDER_DBG_FILE"

Probably don't need these as #defines

> +#define PRINT_ERR(fmt, ...)								\
> +do {											\
> +	fprintf(stderr, "[%s:%d]" fmt, __func__, __LINE__, ##__VA_ARGS__);		\
> +} while (0)
> +
> +#define PRINT_DBG(dbg_mask, dbg_level, fmt, ...)					\
> +do {											\
> +	if ((rdma_dbg_mask & dbg_mask) && (rdma_dbg_level >= dbg_level))		\
> +		fprintf(stderr, "[%s:%d]" fmt, __func__, __LINE__, ##__VA_ARGS__);	\
> +} while (0)

This could just be LOG_DBG(stderr, ....)

> +
> +#define LOG_DBG(dbg_fp, dbg_mask, dbg_level, fmt, ...)					\
> +do {											\
> +	if ((rdma_dbg_mask & dbg_mask) && (rdma_dbg_level >= dbg_level))		\
> +		fprintf(dbg_fp, "[%s:%d]" fmt, __func__, __LINE__, ##__VA_ARGS__);	\
> +} while (0)
> +
> +#define LOG_DBG_FLUSH(dbg_fp, dbg_mask, dbg_level, fmt, ...)				\
> +do {											\
> +		LOG_DBG(dbg_fp, dbg_mask, dbg_level, fmt, ##__VA_ARGS__);		\
> +		if (dbg_fp != stderr)							\
> +			fflush(dbg_fp);							\
> +} while (0)

Hum, wondering if it is worth retaining this FLUSH behavior or if we
should just have a way to open the log file with line buffering?

This patch should also include man page content please.

I think people will ask for the 'MLX5_DEBUG' style total compile out.
Should we have a --with-debug-log cmake option and disable it by
default?

'setup_debug' should probably happen someplace in verbs, not be in
every driver.

Otherwise looks essentially reasonable to me.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html