Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200707003937.187125-1-alainm@chromium.org>
In-Reply-To: <20200707003937.187125-1-alainm@chromium.org>
From:   Alain Michaud <alainmichaud@google.com>
Date:   Mon, 6 Jul 2020 22:22:09 -0400
Message-ID: <CALWDO_Ut-k0WDTkA-o6uj+S1ctEUQu-mXcC3M0oVgMdczo+mZw@mail.gmail.com>
Subject: Re: [PATCH v1] Bluetooth: create CONFIG_BT_DEBUG_FUNC_NAME
To:     Alain Michaud <alainm@chromium.org>
Cc:     BlueZ <linux-bluetooth@vger.kernel.org>,
        Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

This feature depends (soft dependency) on the kernel experimental
feature defined in this patch:
https://patchwork.kernel.org/patch/11646081/

On Mon, Jul 6, 2020 at 8:39 PM Alain Michaud <alainm@chromium.org> wrote:
>
> Creates a CONFIG_BT_DEBUG_FUNC_NAME option to include function names in
> debug statements.
>
> Unlike other platforms __func__ isn't a string literal so it cannot be
> automatically concatenated by the pre-processor.  As a result, the
> function name is passed as a parameter to the tracing function.  Since
> pr_debug is a function like macro, the normal expansion of BT_PREFIX_PARAM
> does not work as it gets processed within the first parameter as well,
> for this reason, BT_DBG is split into two versions.
>
> This patch was built tested with all 4 possible combinations of
> CONFIG_BT_DEBUG_FUNC_NAME and CONFIG_BT_FEATURE_DEBUG configurations.
>
> Signed-off-by: Alain Michaud <alainm@chromium.org>
> Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> ---
>
>  include/net/bluetooth/bluetooth.h | 32 +++++++++++++++++++++++--------
>  net/bluetooth/Kconfig             | 11 +++++++++++
>  2 files changed, 35 insertions(+), 8 deletions(-)
>
> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> index 7ee8041af803..27ec8f2a7c28 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -162,22 +162,37 @@ void bt_dbg_set(bool enable);
>  bool bt_dbg_get(void);
>  __printf(1, 2)
>  void bt_dbg(const char *fmt, ...);
> +#define BT_DBG_INT     bt_dbg
> +#else
> +#define BT_DBG_INT     pr_debug
>  #endif
>  __printf(1, 2)
>  void bt_warn_ratelimited(const char *fmt, ...);
>  __printf(1, 2)
>  void bt_err_ratelimited(const char *fmt, ...);
>
> -#define BT_INFO(fmt, ...)      bt_info(fmt "\n", ##__VA_ARGS__)
> -#define BT_WARN(fmt, ...)      bt_warn(fmt "\n", ##__VA_ARGS__)
> -#define BT_ERR(fmt, ...)       bt_err(fmt "\n", ##__VA_ARGS__)
> -
> -#if IS_ENABLED(CONFIG_BT_FEATURE_DEBUG)
> -#define BT_DBG(fmt, ...)       bt_dbg(fmt "\n", ##__VA_ARGS__)
> +#if IS_ENABLED(CONFIG_BT_DEBUG_FUNC_NAMES)
> +#define BT_PREFIX "%s() "
> +#define BT_PREFIX_PARAM ,__func__
> +#define BT_DBG(fmt, ...)       \
> +       BT_DBG_INT(BT_PREFIX fmt "\n", __func__, ##__VA_ARGS__)
>  #else
> -#define BT_DBG(fmt, ...)       pr_debug(fmt "\n", ##__VA_ARGS__)
> +#define BT_PREFIX
> +#define BT_PREFIX_PARAM
> +#define BT_DBG(fmt, ...)       \
> +       BT_DBG_INT(fmt "\n", ##__VA_ARGS__)
>  #endif
>
> +#define BT_INFO(fmt, ...)      \
> +       bt_info(BT_PREFIX fmt "\n" BT_PREFIX_PARAM, ##__VA_ARGS__)
> +#define BT_WARN(fmt, ...)      \
> +       bt_warn(BT_PREFIX fmt "\n" BT_PREFIX_PARAM, ##__VA_ARGS__)
> +#define BT_ERR(fmt, ...)       \
> +       bt_err(BT_PREFIX fmt "\n" BT_PREFIX_PARAM, ##__VA_ARGS__)
> +
> +#define BT_ERR_RATELIMITED(fmt, ...) \
> +       bt_err_ratelimited(BT_PREFIX fmt "\n" BT_PREFIX_PARAM, ##__VA_ARGS__)
> +
>  #define bt_dev_info(hdev, fmt, ...)                            \
>         BT_INFO("%s: " fmt, (hdev)->name, ##__VA_ARGS__)
>  #define bt_dev_warn(hdev, fmt, ...)                            \
> @@ -188,7 +203,8 @@ void bt_err_ratelimited(const char *fmt, ...);
>         BT_DBG("%s: " fmt, (hdev)->name, ##__VA_ARGS__)
>
>  #define bt_dev_warn_ratelimited(hdev, fmt, ...)                        \
> -       bt_warn_ratelimited("%s: " fmt, (hdev)->name, ##__VA_ARGS__)
> +       bt_warn_ratelimited("%s: " BT_PREFIX fmt, (hdev)->name  \
> +                           BT_PREFIX_PARAM, ##__VA_ARGS__)
>  #define bt_dev_err_ratelimited(hdev, fmt, ...)                 \
>         bt_err_ratelimited("%s: " fmt, (hdev)->name, ##__VA_ARGS__)
>
> diff --git a/net/bluetooth/Kconfig b/net/bluetooth/Kconfig
> index 1d6d243cdde9..de31c682c7b0 100644
> --- a/net/bluetooth/Kconfig
> +++ b/net/bluetooth/Kconfig
> @@ -142,4 +142,15 @@ config BT_FEATURE_DEBUG
>           This provides an option to enable/disable debugging statements
>           at runtime via the experimental features interface.
>
> +config BT_DEBUG_FUNC_NAMES
> +       bool "Include function names in debugging statements"
> +       depends on BT
> +       default n
> +       help
> +         Provides an option to include function names in debugging
> +         statements.
> +
> +         When enabled, trace statements will include the function name as a
> +         prefix which may help identify the source code references.
> +
>  source "drivers/bluetooth/Kconfig"
> --
> 2.27.0.212.ge8ba1cc988-goog
>