linux-kernel - Re: [PATCH V2 6/6] ceph_debug: Remove now unused dout macro definitions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 17 Aug 2020 11:24:14 +0200
From:   Ilya Dryomov <idryomov@...il.com>
To:     Joe Perches <joe@...ches.com>
Cc:     Jeff Layton <jlayton@...nel.org>,
        Ceph Development <ceph-devel@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V2 6/6] ceph_debug: Remove now unused dout macro definitions

On Mon, Aug 17, 2020 at 3:34 AM Joe Perches <joe@...ches.com> wrote:
>
> All the uses have be converted to pr_debug, so remove these.
>
> Signed-off-by: Joe Perches <joe@...ches.com>
> ---
>  include/linux/ceph/ceph_debug.h | 30 ------------------------------
>  1 file changed, 30 deletions(-)
>
> diff --git a/include/linux/ceph/ceph_debug.h b/include/linux/ceph/ceph_debug.h
> index d5a5da838caf..81c0d7195f1e 100644
> --- a/include/linux/ceph/ceph_debug.h
> +++ b/include/linux/ceph/ceph_debug.h
> @@ -6,34 +6,4 @@
>
>  #include <linux/string.h>
>
> -#ifdef CONFIG_CEPH_LIB_PRETTYDEBUG
> -
> -/*
> - * wrap pr_debug to include a filename:lineno prefix on each line.
> - * this incurs some overhead (kernel size and execution time) due to
> - * the extra function call at each call site.
> - */
> -
> -# if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
> -#  define dout(fmt, ...)                                               \
> -       pr_debug("%.*s %12.12s:%-4d : " fmt,                            \
> -                8 - (int)sizeof(KBUILD_MODNAME), "    ",               \
> -                kbasename(__FILE__), __LINE__, ##__VA_ARGS__)
> -# else
> -/* faux printk call just to see any compiler warnings. */
> -#  define dout(fmt, ...)       do {                            \
> -               if (0)                                          \
> -                       printk(KERN_DEBUG fmt, ##__VA_ARGS__);  \
> -       } while (0)
> -# endif
> -
> -#else
> -
> -/*
> - * or, just wrap pr_debug
> - */
> -# define dout(fmt, ...)        pr_debug(" " fmt, ##__VA_ARGS__)
> -
> -#endif
> -
>  #endif
> --
> 2.26.0
>

Hi Joe,

Yeah, roughly the same thing can be achieved with +flmp instead
of just +p with PRETTYDEBUG, but PRETTYDEBUG formatting actually
predates those flags and some of us still use bash scripts from
back then.  We also have a few guides and blog entries with just
+p, but that's not a big deal.

I'd be fine with removing CONFIG_CEPH_LIB_PRETTYDEBUG since it's
disabled by default and in all major distributions, but I'm not a
fan of a wide-sweeping dout -> pr_debug change.  We do extensive
backporting to older kernels and these kind of changes are rather
annoying.  dout is shorter to type too ;)

I know that in some cases the function names are outdated or
duplicated, but I prefer fixing them gradually, along with actual
code changes in the area (i.e. similar to whitespace).

Thanks,

                Ilya