Messages in this thread | | | From | Orson Zhai <> | Date | Tue, 31 Mar 2020 07:44:55 +0800 | Subject | Re: [RFC PATCH V2] dynamic_debug: Add config option of DYNAMIC_DEBUG_CORE |
| |
Hi Jason,
On Mon, Mar 30, 2020 at 11:19 PM Jason Baron <jbaron@akamai.com> wrote: > > > > On 3/28/20 2:17 AM, Orson Zhai wrote: > > Hi Jason, > > > > On Thu, Mar 26, 2020 at 1:13 AM Orson Zhai <orson.unisoc@gmail.com> wrote: > >> > >> Instead of enabling whole kernel with CONFIG_DYNAMIC_DEBUG, CONFIG_ > >> DYNAMIC_DEBUG_CORE will only make the basic function definition and > >> exported symbols to be built without replacing pr_debug or dev_dbg > >> to dynamic version unless DEBUG is defined for any desired modules > > > > How do you think about this idea? > > Optionally enable dynamic debug of modules by the DEBUG macro. > > > > Hi Orson, > > So I like the idea of being able to use pr_debug() as a way for modules to > tie into dynamic_debug() without having to enable everything via > CONFIG_DYNAMIC_DEBUG. However the 'DEBUG' already has a specific meaning > which is to enable the printing or enable by default (can be turned off > at run-time if CONFIG_DYNAMIC_DEBUG is set).
You are right. We'd better not to break user's experience in their mind by new added feature.
> > So I think generally modules want the printing off by default. So maybe > what we want is instead of: > > >> +#if defined(CONFIG_DYNAMIC_DEBUG) || \ > >> + (defined(CONFIG_DYNAMIC_CORE) && defined(DEBUG)) > > is: > > >> +#if defined(CONFIG_DYNAMIC_DEBUG) || \ > >> + (defined(CONFIG_DYNAMIC_CORE) && defined(DEBUG_MODULE)) > > > That is introduce a new per-module definition I've called it > 'DEBUG_MODULE' (perhaps we can name it better), that enables
How about use DYNDBG for this? It will be consistent with dyndbg in boot command line parameters.
Best, -Orson
> dynamic debugging without having to turn it on globally. Then, > modules can in addition enabled 'DEBUG' if they want all the printing > on by default. > > Thanks, > > -Jason > > > > Best Regards, > > -Orson > > > >> together by users. > >> > >> This is useful for people who only want to enable dynamic debug for some > >> specific kernel modules without worrying about whole kernel image size will > >> be significantly increased and more memory consumption caused by CONFIG_ > >> DYNAMIC_DEBUG. > >> > >> Signed-off-by: Orson Zhai <orson.unisoc@gmail.com> > >> --- > >> Changes from V1: > >> - Rewrite commit message for more generic usage. > >> - Add combination use of CONFIG_DYNAMIC_DEBUG_CORE and DEBUG to enable > >> dynamic debug seperately. > >> - Ignore empty _ddtable and return success when only the core is enabled. > >> - Fix all typoes. > >> > >> include/linux/dev_printk.h | 6 ++++-- > >> include/linux/dynamic_debug.h | 2 +- > >> include/linux/printk.h | 12 ++++++++---- > >> lib/Kconfig.debug | 18 ++++++++++++++++++ > >> lib/Makefile | 2 +- > >> lib/dynamic_debug.c | 9 +++++++-- > >> 6 files changed, 39 insertions(+), 10 deletions(-) > >> > >> diff --git a/include/linux/dev_printk.h b/include/linux/dev_printk.h > >> index 5aad06b..ed40030 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_CORE) && defined(DEBUG)) > >> #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_CORE) && defined(DEBUG)) > >> /* 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..44d5378 100644 > >> --- a/include/linux/printk.h > >> +++ b/include/linux/printk.h > >> @@ -292,7 +292,8 @@ 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. > >> + * or CONFIG_DYNAMIC_DEBUG is set, or both CONFIG_DYNAMIC_DEBUG_CORE and > >> + * DEBUG is defined. > >> */ > >> #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_CORE) && defined(DEBUG)) > >> #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_CORE) && defined(DEBUG)) > >> /* 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_CORE) && defined(DEBUG)) > >> #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 69def4a..8381b19 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 > >> + select DYNAMIC_DEBUG_CORE > >> help > >> > >> Compiles debug level messages into the kernel, which would not > >> @@ -164,6 +165,23 @@ config DYNAMIC_DEBUG > >> See Documentation/admin-guide/dynamic-debug-howto.rst for additional > >> information. > >> > >> +config DYNAMIC_DEBUG_CORE > >> + bool "Enable core functions of dynamic debug support" > >> + depends on PRINTK > >> + depends on DEBUG_FS > >> + help > >> + Enable this option to build ddebug_* and __dynamic_* routines > >> + into kernel. If you want enable whole dynamic debug features, > >> + select CONFIG_DYNAMIC_DEBUG directly and this option will be > >> + automatically selected as well. > >> + > >> + This option can be selected with DEBUG together which could be > >> + defined for desired kernel modules to enable dynamic debug > >> + features instead for whole kernel. Especially being used in > >> + the case that kernel modules are built out of kernel tree to > >> + have dynamic debug capabilities without affecting the kernel > >> + base. > >> + > >> config SYMBOLIC_ERRNAME > >> bool "Support symbolic error names in printf" > >> default y if PRINTK > >> diff --git a/lib/Makefile b/lib/Makefile > >> index 611872c..2096d83 100644 > >> --- a/lib/Makefile > >> +++ b/lib/Makefile > >> @@ -183,7 +183,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 c604091..34f303a 100644 > >> --- a/lib/dynamic_debug.c > >> +++ b/lib/dynamic_debug.c > >> @@ -1014,8 +1014,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 core only build\n"); > >> + ddebug_init_success = 1; > >> + return 0; > >> } > >> iter = __start___verbose; > >> modname = iter->modname; > >> -- > >> 2.7.4 > >>
| |