2020-03-25 17:13:50

by Orson Zhai

[permalink] [raw]
Subject: [RFC PATCH V2] dynamic_debug: Add config option of DYNAMIC_DEBUG_CORE

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
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 <[email protected]>
---
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


2020-03-28 06:18:38

by Orson Zhai

[permalink] [raw]
Subject: Re: [RFC PATCH V2] dynamic_debug: Add config option of DYNAMIC_DEBUG_CORE

Hi Jason,

On Thu, Mar 26, 2020 at 1:13 AM Orson Zhai <[email protected]> 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.

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 <[email protected]>
> ---
> 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
>

2020-03-30 16:30:15

by Jason Baron

[permalink] [raw]
Subject: Re: [RFC PATCH V2] dynamic_debug: Add config option of DYNAMIC_DEBUG_CORE



On 3/28/20 2:17 AM, Orson Zhai wrote:
> Hi Jason,
>
> On Thu, Mar 26, 2020 at 1:13 AM Orson Zhai <[email protected]> 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).

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
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 <[email protected]>
>> ---
>> 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
>>

2020-03-30 23:45:57

by Orson Zhai

[permalink] [raw]
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 <[email protected]> wrote:
>
>
>
> On 3/28/20 2:17 AM, Orson Zhai wrote:
> > Hi Jason,
> >
> > On Thu, Mar 26, 2020 at 1:13 AM Orson Zhai <[email protected]> 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 <[email protected]>
> >> ---
> >> 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
> >>