Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <cover.1597626802.git.joe@perches.com> <fe4e9e2a544af4d00aa5c97d9288d9b4d67db1ed.1597626802.git.joe@perches.com>
In-Reply-To: <fe4e9e2a544af4d00aa5c97d9288d9b4d67db1ed.1597626802.git.joe@perches.com>
From:   Ilya Dryomov <idryomov@gmail.com>
Date:   Mon, 17 Aug 2020 11:24:14 +0200
Message-ID: <CAOi1vP8rL6R8LqFnz10O=N4gfC2zYiZdfZbg3NkcePRvOKu7xA@mail.gmail.com>
Subject: Re: [PATCH V2 6/6] ceph_debug: Remove now unused dout macro definitions
To:     Joe Perches <joe@perches.com>
Cc:     Jeff Layton <jlayton@kernel.org>,
        Ceph Development <ceph-devel@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Aug 17, 2020 at 3:34 AM Joe Perches <joe@perches.com> wrote:
>
> All the uses have be converted to pr_debug, so remove these.
>
> Signed-off-by: Joe Perches <joe@perches.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