All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH V2] dynamic_debug: Add config option of DYNAMIC_DEBUG_CORE
@ 2020-03-25 17:12 Orson Zhai
  2020-03-28  6:17 ` Orson Zhai
  0 siblings, 1 reply; 4+ messages in thread
From: Orson Zhai @ 2020-03-25 17:12 UTC (permalink / raw)
  To: Jason Baron, Andrew Morton, Randy Dunlap, Masahiro Yamada,
	Shuah Khan, Krzysztof Kozlowski, Masami Hiramatsu,
	Brendan Higgins, Herbert Xu, Ard Biesheuvel, Andy Shevchenko,
	David Gow, Mark Rutland, joe
  Cc: orsonzhai, linux-kernel, kernel-team, Orson Zhai

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 <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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [RFC PATCH V2] dynamic_debug: Add config option of DYNAMIC_DEBUG_CORE
  2020-03-25 17:12 [RFC PATCH V2] dynamic_debug: Add config option of DYNAMIC_DEBUG_CORE Orson Zhai
@ 2020-03-28  6:17 ` Orson Zhai
  2020-03-30 15:19   ` Jason Baron
  0 siblings, 1 reply; 4+ messages in thread
From: Orson Zhai @ 2020-03-28  6:17 UTC (permalink / raw)
  To: Jason Baron
  Cc: Andrew Morton, Randy Dunlap, Masahiro Yamada, Shuah Khan,
	Krzysztof Kozlowski, Masami Hiramatsu, Brendan Higgins,
	Herbert Xu, Ard Biesheuvel, Andy Shevchenko, David Gow,
	Mark Rutland, joe, Linux Kernel Mailing List,
	Android Kernel Team, Orson Zhai

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.

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
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC PATCH V2] dynamic_debug: Add config option of DYNAMIC_DEBUG_CORE
  2020-03-28  6:17 ` Orson Zhai
@ 2020-03-30 15:19   ` Jason Baron
  2020-03-30 23:44     ` Orson Zhai
  0 siblings, 1 reply; 4+ messages in thread
From: Jason Baron @ 2020-03-30 15:19 UTC (permalink / raw)
  To: Orson Zhai
  Cc: Andrew Morton, Randy Dunlap, Masahiro Yamada, Shuah Khan,
	Krzysztof Kozlowski, Masami Hiramatsu, Brendan Higgins,
	Herbert Xu, Ard Biesheuvel, Andy Shevchenko, David Gow,
	Mark Rutland, joe, Linux Kernel Mailing List,
	Android Kernel Team, Orson Zhai



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).

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 <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
>>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC PATCH V2] dynamic_debug: Add config option of DYNAMIC_DEBUG_CORE
  2020-03-30 15:19   ` Jason Baron
@ 2020-03-30 23:44     ` Orson Zhai
  0 siblings, 0 replies; 4+ messages in thread
From: Orson Zhai @ 2020-03-30 23:44 UTC (permalink / raw)
  To: Jason Baron
  Cc: Andrew Morton, Randy Dunlap, Masahiro Yamada, Shuah Khan,
	Krzysztof Kozlowski, Masami Hiramatsu, Brendan Higgins,
	Herbert Xu, Ard Biesheuvel, Andy Shevchenko, David Gow,
	Mark Rutland, joe, Linux Kernel Mailing List,
	Android Kernel Team, Orson Zhai

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
> >>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-03-30 23:45 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-25 17:12 [RFC PATCH V2] dynamic_debug: Add config option of DYNAMIC_DEBUG_CORE Orson Zhai
2020-03-28  6:17 ` Orson Zhai
2020-03-30 15:19   ` Jason Baron
2020-03-30 23:44     ` Orson Zhai

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.