Re: [PATCH] dynamic_debug: Add an option to enable dynamic debug for modules only

From: Orson Zhai
Date: Wed Apr 15 2020 - 22:02:01 EST

Next message: Hugh Dickins: "Re: possible deadlock in shmem_uncharge"
Previous message: Tianjia Zhang: "Re: [PATCH] KVM: Optimize kvm_arch_vcpu_ioctl_run function"
In reply to: Jason Baron: "Re: [PATCH] dynamic_debug: Add an option to enable dynamic debug for modules only"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Thu, Apr 16, 2020 at 2:48 AM Jason Baron <jbaron@xxxxxxxxxx> wrote:
>
>
>
> On 4/15/20 12:31 PM, Orson Zhai wrote:
> > Hi Jason,
> >
> > On Fri, Apr 10, 2020 at 8:33 PM Orson Zhai <orson.unisoc@xxxxxxxxx> wrote:
> >>
> >> From: Orson Zhai <orson.zhai@xxxxxxxxxx>
> >>
> >> Instead of enabling dynamic debug globally with CONFIG_DYNAMIC_DEBUG,
> >> CONFIG_DYNAMIC_DEBUG_CORE will only enable core function of dynamic
> >> debug. With the DEBUG_MODULE defined for any modules, dynamic debug
> >> will be tied to them.
> >>
> >> This is useful for people who only want to enable dynamic debug for
> >> kernel modules without worrying about kernel image size and memory
> >> consumption is increasing too much.
> >>
> >
> > Do you have any comments for this patch?
> >
>
> So I like that you added the 'DEBUG_MODULE', but I'm wondering if the
> naming is too generic? Maybe DYNAMIC_DEBUG_MODULE?

OK, I think it is more precise than before.

>
> There may also be other places where you want to have the per-module
> dependency, for example netdev_dbg()? But perhaps further expansion
> could wait for a user...

Yes.
I find more in netdevice.h, net.h and ib_verbs.h.
I will change all of them in V2.
But acpi.h seems different which I'd like to ignore.

Best,
Orson

>
> Thanks,
>
> -Jason
>
>
> > Best Regards,
> > Orson
> >
> >> Signed-off-by: Orson Zhai <orson.zhai@xxxxxxxxxx>
> >> ---
> >> Documentation/admin-guide/dynamic-debug-howto.rst | 7 +++++--
> >> include/linux/dev_printk.h | 6 ++++--
> >> include/linux/dynamic_debug.h | 2 +-
> >> include/linux/printk.h | 14 +++++++++-----
> >> lib/Kconfig.debug | 12 ++++++++++++
> >> lib/Makefile | 2 +-
> >> lib/dynamic_debug.c | 9 +++++++--
> >> 7 files changed, 39 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
> >> index 0dc2eb8..fa5b8d4 100644
> >> --- a/Documentation/admin-guide/dynamic-debug-howto.rst
> >> +++ b/Documentation/admin-guide/dynamic-debug-howto.rst
> >> @@ -13,8 +13,11 @@ kernel code to obtain additional kernel information. Currently, if
> >> ``print_hex_dump_debug()``/``print_hex_dump_bytes()`` calls can be dynamically
> >> enabled per-callsite.
> >>
> >> -If ``CONFIG_DYNAMIC_DEBUG`` is not set, ``print_hex_dump_debug()`` is just
> >> -shortcut for ``print_hex_dump(KERN_DEBUG)``.
> >> +If ``CONFIG_DYNAMIC_DEBUG_CORE`` is set, only the modules with ``DEBUG_MODULE``
> >> +defined will be tied into dynamic debug.
> >> +
> >> +If ``CONFIG_DYNAMIC_DEBUG`` or ``CONFIG_DYNAMIC_DEBUG_CORE`` is not set,
> >> +``print_hex_dump_debug()`` is just shortcut for ``print_hex_dump(KERN_DEBUG)``.
> >>
> >> For ``print_hex_dump_debug()``/``print_hex_dump_bytes()``, format string is
> >> its ``prefix_str`` argument, if it is constant string; or ``hexdump``
> >> diff --git a/include/linux/dev_printk.h b/include/linux/dev_printk.h
> >> index 5aad06b..2fb0671 100644
> >> --- a/include/linux/dev_printk.h
> >> +++ b/include/linux/dev_printk.h
> >> @@ -109,7 +109,8 @@ void _dev_info(const struct device *dev, const char *fmt, ...)
> >> #define dev_info(dev, fmt, ...) \
> >> _dev_info(dev, dev_fmt(fmt), ##__VA_ARGS__)
> >>
> >> -#if defined(CONFIG_DYNAMIC_DEBUG)
> >> +#if defined(CONFIG_DYNAMIC_DEBUG) || \
> >> + (defined(CONFIG_DYNAMIC_DEBUG_CORE) && defined(DEBUG_MODULE))
> >> #define dev_dbg(dev, fmt, ...) \
> >> dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
> >> #elif defined(DEBUG)
> >> @@ -181,7 +182,8 @@ do { \
> >> dev_level_ratelimited(dev_notice, dev, fmt, ##__VA_ARGS__)
> >> #define dev_info_ratelimited(dev, fmt, ...) \
> >> dev_level_ratelimited(dev_info, dev, fmt, ##__VA_ARGS__)
> >> -#if defined(CONFIG_DYNAMIC_DEBUG)
> >> +#if defined(CONFIG_DYNAMIC_DEBUG) || \
> >> + (defined(CONFIG_DYNAMIC_DEBUG_CORE) && defined(DEBUG_MODULE))
> >> /* descriptor check is first to prevent flooding with "callbacks suppressed" */
> >> #define dev_dbg_ratelimited(dev, fmt, ...) \
> >> do { \
> >> diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
> >> index 4cf02ec..abcd5fd 100644
> >> --- a/include/linux/dynamic_debug.h
> >> +++ b/include/linux/dynamic_debug.h
> >> @@ -48,7 +48,7 @@ struct _ddebug {
> >>
> >>
> >>
> >> -#if defined(CONFIG_DYNAMIC_DEBUG)
> >> +#if defined(CONFIG_DYNAMIC_DEBUG_CORE)
> >> int ddebug_add_module(struct _ddebug *tab, unsigned int n,
> >> const char *modname);
> >> extern int ddebug_remove_module(const char *mod_name);
> >> diff --git a/include/linux/printk.h b/include/linux/printk.h
> >> index 1e6108b..77fab5b 100644
> >> --- a/include/linux/printk.h
> >> +++ b/include/linux/printk.h
> >> @@ -291,8 +291,9 @@ extern int kptr_restrict;
> >> /*
> >> * These can be used to print at the various log levels.
> >> * All of these will print unconditionally, although note that pr_debug()
> >> - * and other debug macros are compiled out unless either DEBUG is defined
> >> - * or CONFIG_DYNAMIC_DEBUG is set.
> >> + * and other debug macros are compiled out unless either DEBUG is defined,
> >> + * CONFIG_DYNAMIC_DEBUG is set, or CONFIG_DYNAMIC_DEBUG_CORE is set when
> >> + * DEBUG_MODULE being defined for any modules.
> >> */
> >> #define pr_emerg(fmt, ...) \
> >> printk(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
> >> @@ -327,7 +328,8 @@ extern int kptr_restrict;
> >>
> >>
> >> /* If you are writing a driver, please use dev_dbg instead */
> >> -#if defined(CONFIG_DYNAMIC_DEBUG)
> >> +#if defined(CONFIG_DYNAMIC_DEBUG) || \
> >> + (defined(CONFIG_DYNAMIC_DEBUG_CORE) && defined(DEBUG_MODULE))
> >> #include <linux/dynamic_debug.h>
> >>
> >> /* dynamic_pr_debug() uses pr_fmt() internally so we don't need it here */
> >> @@ -453,7 +455,8 @@ extern int kptr_restrict;
> >> #endif
> >>
> >> /* If you are writing a driver, please use dev_dbg instead */
> >> -#if defined(CONFIG_DYNAMIC_DEBUG)
> >> +#if defined(CONFIG_DYNAMIC_DEBUG) || \
> >> + (defined(CONFIG_DYNAMIC_DEBUG_CORE) && defined(DEBUG_MODULE))
> >> /* descriptor check is first to prevent flooding with "callbacks suppressed" */
> >> #define pr_debug_ratelimited(fmt, ...) \
> >> do { \
> >> @@ -500,7 +503,8 @@ static inline void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
> >>
> >> #endif
> >>
> >> -#if defined(CONFIG_DYNAMIC_DEBUG)
> >> +#if defined(CONFIG_DYNAMIC_DEBUG) || \
> >> + (defined(CONFIG_DYNAMIC_DEBUG_CORE) && defined(DEBUG_MODULE))
> >> #define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \
> >> groupsize, buf, len, ascii) \
> >> dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
> >> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> >> index 50c1f5f..25a1b9de 100644
> >> --- a/lib/Kconfig.debug
> >> +++ b/lib/Kconfig.debug
> >> @@ -99,6 +99,7 @@ config DYNAMIC_DEBUG
> >> default n
> >> depends on PRINTK
> >> depends on (DEBUG_FS || PROC_FS)
> >> + select DYNAMIC_DEBUG_CORE
> >> help
> >>
> >> Compiles debug level messages into the kernel, which would not
> >> @@ -165,6 +166,17 @@ config DYNAMIC_DEBUG
> >> See Documentation/admin-guide/dynamic-debug-howto.rst for additional
> >> information.
> >>
> >> +config DYNAMIC_DEBUG_CORE
> >> + bool "Enable core function of dynamic debug support"
> >> + depends on PRINTK
> >> + depends on (DEBUG_FS || PROC_FS)
> >> + help
> >> + Enable core functional support of dynamic debug. It is useful
> >> + when you want to tie dynamic debug to your kernel modules with
> >> + DEBUG_MODULE defined for each of them, especially for the case
> >> + of embedded system where the kernel image size is sensitive for
> >> + people.
> >> +
> >> config SYMBOLIC_ERRNAME
> >> bool "Support symbolic error names in printf"
> >> default y if PRINTK
> >> diff --git a/lib/Makefile b/lib/Makefile
> >> index 685aee6..8952772 100644
> >> --- a/lib/Makefile
> >> +++ b/lib/Makefile
> >> @@ -186,7 +186,7 @@ lib-$(CONFIG_GENERIC_BUG) += bug.o
> >>
> >> obj-$(CONFIG_HAVE_ARCH_TRACEHOOK) += syscall.o
> >>
> >> -obj-$(CONFIG_DYNAMIC_DEBUG) += dynamic_debug.o
> >> +obj-$(CONFIG_DYNAMIC_DEBUG_CORE) += dynamic_debug.o
> >> obj-$(CONFIG_SYMBOLIC_ERRNAME) += errname.o
> >>
> >> obj-$(CONFIG_NLATTR) += nlattr.o
> >> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> >> index 8f199f4..321437b 100644
> >> --- a/lib/dynamic_debug.c
> >> +++ b/lib/dynamic_debug.c
> >> @@ -1032,8 +1032,13 @@ static int __init dynamic_debug_init(void)
> >> int verbose_bytes = 0;
> >>
> >> if (&__start___verbose == &__stop___verbose) {
> >> - pr_warn("_ddebug table is empty in a CONFIG_DYNAMIC_DEBUG build\n");
> >> - return 1;
> >> + if (IS_ENABLED(CONFIG_DYNAMIC_DEBUG)) {
> >> + pr_warn("_ddebug table is empty in a CONFIG_DYNAMIC_DEBUG build\n");
> >> + return 1;
> >> + }
> >> + pr_info("Ignore empty _ddebug table in a CONFIG_DYNAMIC_DEBUG_CORE build\n");
> >> + ddebug_init_success = 1;
> >> + return 0;
> >> }
> >> iter = __start___verbose;
> >> modname = iter->modname;
> >> --
> >> 2.7.4
> >>

Next message: Hugh Dickins: "Re: possible deadlock in shmem_uncharge"
Previous message: Tianjia Zhang: "Re: [PATCH] KVM: Optimize kvm_arch_vcpu_ioctl_run function"
In reply to: Jason Baron: "Re: [PATCH] dynamic_debug: Add an option to enable dynamic debug for modules only"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]