[RFC PATCH] dynamic_debug: Add config option of DYNAMIC_DEBUG_CORE

[RFC PATCH] dynamic_debug: Add config option of DYNAMIC_DEBUG_CORE

[Orson Zhai]

There is the requirement from new Android that kernel image (GKI) and
kernel modules are supposed to be built at differnet places. Some people
want to enable dynamic debug for kernel modules only but not for kernel
image itself with the consideration of binary size increased or more
memory being used.

By this patch, dynamic debug is divided into core part (the defination of
functions) and macro replacement part. We can only have the core part to
be built-in and do not have to activate the debug output from kenrel image.

Signed-off-by: Orson Zhai <orson.unisoc@gmail.com>
---
 include/linux/dynamic_debug.h |  2 +-
 lib/Kconfig.debug             | 18 ++++++++++++++++--
 lib/Makefile                  |  2 +-
 3 files changed, 18 insertions(+), 4 deletions(-)

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/lib/Kconfig.debug b/lib/Kconfig.debug
index 69def4a..78a7256 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -97,8 +97,7 @@ config BOOT_PRINTK_DELAY
 config DYNAMIC_DEBUG
 	bool "Enable dynamic printk() support"
 	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 +163,21 @@ 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.
+
+	  This option is selected when you want to enable dynamic debug
+	  for kernel modules only but not for the kernel base. Especailly
+	  in the case that kernel modules are built out of the place where
+	  kernel base is built.
+
 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
-- 
2.7.4

Re: [RFC PATCH] dynamic_debug: Add config option of DYNAMIC_DEBUG_CORE

[Andy Shevchenko]

On Wed, Mar 18, 2020 at 9:04 PM Orson Zhai <orson.unisoc@gmail.com> wrote:
>
> There is the requirement from new Android that kernel image (GKI) and
> kernel modules are supposed to be built at differnet places. Some people
> want to enable dynamic debug for kernel modules only but not for kernel
> image itself with the consideration of binary size increased or more
> memory being used.
>
> By this patch, dynamic debug is divided into core part (the defination of
> functions) and macro replacement part. We can only have the core part to
> be built-in and do not have to activate the debug output from kenrel image.
>

There are few grammar typos in above...

>  config DYNAMIC_DEBUG
>         bool "Enable dynamic printk() support"
>         default n

> -       depends on PRINTK
> -       depends on DEBUG_FS

You may not touch this. By removing them you effectively removed
dependencies :-(

> +       select DYNAMIC_DEBUG_CORE
>         help
>
>           Compiles debug level messages into the kernel, which would not
> @@ -164,6 +163,21 @@ 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.
> +
> +         This option is selected when you want to enable dynamic debug

> +         for kernel modules only but not for the kernel base. Especailly

Typo.

> +         in the case that kernel modules are built out of the place where
> +         kernel base is built.

Highly recommend to ask somebody to do proof read.

-- 
With Best Regards,
Andy Shevchenko

Re: [RFC PATCH] dynamic_debug: Add config option of DYNAMIC_DEBUG_CORE

[Orson Zhai]

On Thu, Mar 19, 2020 at 3:23 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Wed, Mar 18, 2020 at 9:04 PM Orson Zhai <orson.unisoc@gmail.com> wrote:
> >
> > There is the requirement from new Android that kernel image (GKI) and
> > kernel modules are supposed to be built at differnet places. Some people
> > want to enable dynamic debug for kernel modules only but not for kernel
> > image itself with the consideration of binary size increased or more
> > memory being used.
> >
> > By this patch, dynamic debug is divided into core part (the defination of
> > functions) and macro replacement part. We can only have the core part to
> > be built-in and do not have to activate the debug output from kenrel image.
> >
>
> There are few grammar typos in above...

I am very sorry about this. I though check-patch would remind me, but
it seems not. I'll check this carefully next time.

> >  config DYNAMIC_DEBUG
> >         bool "Enable dynamic printk() support"
> >         default n
>
> > -       depends on PRINTK
> > -       depends on DEBUG_FS
>
> You may not touch this. By removing them you effectively removed
> dependencies :-(

OK. I thought dependencies it could be inherited from the selected one.
But I believe you are right. It's not necessary to be removed.
I will add it back at next version.


>
> > +       select DYNAMIC_DEBUG_CORE
> >         help
> >
> >           Compiles debug level messages into the kernel, which would not
> > @@ -164,6 +163,21 @@ 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.
> > +
> > +         This option is selected when you want to enable dynamic debug
>
> > +         for kernel modules only but not for the kernel base. Especailly
>
> Typo.

Will be fixed next version.

>
> > +         in the case that kernel modules are built out of the place where
> > +         kernel base is built.
>
> Highly recommend to ask somebody to do proof read.

Sorry again.

Thanks for your review.

Best Regards,
-Orson
>
> --
> With Best Regards,
> Andy Shevchenko

Re: [RFC PATCH] dynamic_debug: Add config option of DYNAMIC_DEBUG_CORE

[Joe Perches]

On Thu, 2020-03-19 at 04:11 +0800, Orson Zhai wrote:
> On Thu, Mar 19, 2020 at 3:23 AM Andy Shevchenko
[]
> > Highly recommend to ask somebody to do proof read.
> Sorry again.

It's an RFC, no real need to be sorry, many eyeballs etc...

Re: [RFC PATCH] dynamic_debug: Add config option of DYNAMIC_DEBUG_CORE

[Jason Baron]

On 3/18/20 3:03 PM, Orson Zhai wrote:
> There is the requirement from new Android that kernel image (GKI) and
> kernel modules are supposed to be built at differnet places. Some people
> want to enable dynamic debug for kernel modules only but not for kernel
> image itself with the consideration of binary size increased or more
> memory being used.
> 
> By this patch, dynamic debug is divided into core part (the defination of
> functions) and macro replacement part. We can only have the core part to
> be built-in and do not have to activate the debug output from kenrel image.
> 
> Signed-off-by: Orson Zhai <orson.unisoc@gmail.com>

Hi Orson,

I think this is a nice feature. Is the idea then that driver can do
something like:

#if defined(CONFIG_DRIVER_FOO_DEBUG)
#define driver_foo_debug(fmt, ...) \
        dynamic_pr_debug(fmt, ##__VA_ARGS__)
#else
	no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
#enif

And then the Kconfig:

config DYNAMIC_DRIVER_FOO_DEBUG
	bool "Enable dynamic driver foo printk() support"
	select DYNAMIC_DEBUG_CORE


Or did you have something else in mind? Do you have an example
code for the drivers that you mention?

Thanks,

-Jason


> ---
>  include/linux/dynamic_debug.h |  2 +-
>  lib/Kconfig.debug             | 18 ++++++++++++++++--
>  lib/Makefile                  |  2 +-
>  3 files changed, 18 insertions(+), 4 deletions(-)
> 
> 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/lib/Kconfig.debug b/lib/Kconfig.debug
> index 69def4a..78a7256 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -97,8 +97,7 @@ config BOOT_PRINTK_DELAY
>  config DYNAMIC_DEBUG
>  	bool "Enable dynamic printk() support"
>  	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 +163,21 @@ 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.
> +
> +	  This option is selected when you want to enable dynamic debug
> +	  for kernel modules only but not for the kernel base. Especailly
> +	  in the case that kernel modules are built out of the place where
> +	  kernel base is built.
> +
>  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
> 

Re: [RFC PATCH] dynamic_debug: Add config option of DYNAMIC_DEBUG_CORE

[Orson Zhai]

Hi Jason,

On Wed, Mar 18, 2020 at 05:18:43PM -0400, Jason Baron wrote:
> 
> 
> On 3/18/20 3:03 PM, Orson Zhai wrote:
> > There is the requirement from new Android that kernel image (GKI) and
> > kernel modules are supposed to be built at differnet places. Some people
> > want to enable dynamic debug for kernel modules only but not for kernel
> > image itself with the consideration of binary size increased or more
> > memory being used.
> > 
> > By this patch, dynamic debug is divided into core part (the defination of
> > functions) and macro replacement part. We can only have the core part to
> > be built-in and do not have to activate the debug output from kenrel image.
> > 
> > Signed-off-by: Orson Zhai <orson.unisoc@gmail.com>
> 
> Hi Orson,
> 
> I think this is a nice feature. Is the idea then that driver can do
> something like:
> 
> #if defined(CONFIG_DRIVER_FOO_DEBUG)
> #define driver_foo_debug(fmt, ...) \
>         dynamic_pr_debug(fmt, ##__VA_ARGS__)
> #else
> 	no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> #enif
> 
> And then the Kconfig:
> 
> config DYNAMIC_DRIVER_FOO_DEBUG
> 	bool "Enable dynamic driver foo printk() support"
> 	select DYNAMIC_DEBUG_CORE
> 
I highly appreciate you for giving this good example to us.
To be honest I did not really think of this kind of usage. :)
But it makes much sense. I think dynamic debug might be a little
bit high for requirement of memory. Every line of pr_debug will be
added with a static data structure and malloc with an item in link table.
It might be sensitive especially in embeded system.
So this example shows how to avoid to turn on dynamci debug for whole
system but part of it when being needed.

> 
> Or did you have something else in mind? Do you have an example
> code for the drivers that you mention?

My motivation comes from new Andorid GKI release flow. Android kernel team will
be in charge of GKI release. And SoC vendors will build their device driver as
kernel modules which are diffrent from each vendor. End-users will get their phones
installed with GKI plus some modules all together.

So at Google side, they can only set DYNAMIC_DEBUG_CORE in their defconfig to build
out GKI without worrying about the kernel image size increased too much. Actually
GKI is relatively stable as a common binary and there is no strong reason to do 
dynamic debugging to it.

And at vendor side, they will use a local defconfig which is same with Google one but add 
CONFIG_DYNAMIC_DEBUG to build their kenrel modules. As DYNAMIC_DEBUG enables only a
set of macro expansion, so it has no impact to kernel ABI or the modversion.
All modules will be compatible with GKI and with dynamic debug enabled.

Then the result will be that Google has his clean GKI and vendors have their dynamic-debug-powered modules.

Best Regards,
-Orson

> 
> Thanks,
> 
> -Jason
> 
> 
> > ---
> >  include/linux/dynamic_debug.h |  2 +-
> >  lib/Kconfig.debug             | 18 ++++++++++++++++--
> >  lib/Makefile                  |  2 +-
> >  3 files changed, 18 insertions(+), 4 deletions(-)
> > 
> > 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/lib/Kconfig.debug b/lib/Kconfig.debug
> > index 69def4a..78a7256 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -97,8 +97,7 @@ config BOOT_PRINTK_DELAY
> >  config DYNAMIC_DEBUG
> >  	bool "Enable dynamic printk() support"
> >  	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 +163,21 @@ 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.
> > +
> > +	  This option is selected when you want to enable dynamic debug
> > +	  for kernel modules only but not for the kernel base. Especailly
> > +	  in the case that kernel modules are built out of the place where
> > +	  kernel base is built.
> > +
> >  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
> > 

Re: [RFC PATCH] dynamic_debug: Add config option of DYNAMIC_DEBUG_CORE

[Jason Baron]

On 3/19/20 11:28 AM, Orson Zhai wrote:
> Hi Jason,
> 
> On Wed, Mar 18, 2020 at 05:18:43PM -0400, Jason Baron wrote:
>>
>>
>> On 3/18/20 3:03 PM, Orson Zhai wrote:
>>> There is the requirement from new Android that kernel image (GKI) and
>>> kernel modules are supposed to be built at differnet places. Some people
>>> want to enable dynamic debug for kernel modules only but not for kernel
>>> image itself with the consideration of binary size increased or more
>>> memory being used.
>>>
>>> By this patch, dynamic debug is divided into core part (the defination of
>>> functions) and macro replacement part. We can only have the core part to
>>> be built-in and do not have to activate the debug output from kenrel image.
>>>
>>> Signed-off-by: Orson Zhai <orson.unisoc@gmail.com>
>>
>> Hi Orson,
>>
>> I think this is a nice feature. Is the idea then that driver can do
>> something like:
>>
>> #if defined(CONFIG_DRIVER_FOO_DEBUG)
>> #define driver_foo_debug(fmt, ...) \
>>         dynamic_pr_debug(fmt, ##__VA_ARGS__)
>> #else
>> 	no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
>> #enif
>>
>> And then the Kconfig:
>>
>> config DYNAMIC_DRIVER_FOO_DEBUG
>> 	bool "Enable dynamic driver foo printk() support"
>> 	select DYNAMIC_DEBUG_CORE
>>
> I highly appreciate you for giving this good example to us.
> To be honest I did not really think of this kind of usage. :)
> But it makes much sense. I think dynamic debug might be a little
> bit high for requirement of memory. Every line of pr_debug will be
> added with a static data structure and malloc with an item in link table.
> It might be sensitive especially in embeded system.
> So this example shows how to avoid to turn on dynamci debug for whole
> system but part of it when being needed.
> 
>>
>> Or did you have something else in mind? Do you have an example
>> code for the drivers that you mention?
> 
> My motivation comes from new Andorid GKI release flow. Android kernel team will
> be in charge of GKI release. And SoC vendors will build their device driver as
> kernel modules which are diffrent from each vendor. End-users will get their phones
> installed with GKI plus some modules all together.
> 
> So at Google side, they can only set DYNAMIC_DEBUG_CORE in their defconfig to build
> out GKI without worrying about the kernel image size increased too much. Actually
> GKI is relatively stable as a common binary and there is no strong reason to do 
> dynamic debugging to it.
> 
> And at vendor side, they will use a local defconfig which is same with Google one but add 
> CONFIG_DYNAMIC_DEBUG to build their kenrel modules. As DYNAMIC_DEBUG enables only a
> set of macro expansion, so it has no impact to kernel ABI or the modversion.
> All modules will be compatible with GKI and with dynamic debug enabled.
> 
> Then the result will be that Google has his clean GKI and vendors have their dynamic-debug-powered modules.
>


static int __init dynamic_debug_init(void)
{
        struct _ddebug *iter, *iter_start;
        const char *modname = NULL;
        char *cmdline;
        int ret = 0;
        int n = 0, entries = 0, modct = 0;
        int verbose_bytes = 0;

        if (__start___verbose == __stop___verbose) {
                pr_warn("_ddebug table is empty in a CONFIG_DYNAMIC_DEBUG build\n");
                return 1;
        }

...

I wonder if we should just remove it now.

Thanks,

-Jason

Re: [RFC PATCH] dynamic_debug: Add config option of DYNAMIC_DEBUG_CORE

[Orson Zhai]

On Fri, Mar 20, 2020 at 4:19 AM Jason Baron <jbaron@akamai.com> wrote:
>
>
>
> On 3/19/20 11:28 AM, Orson Zhai wrote:
> > Hi Jason,
> >
> > On Wed, Mar 18, 2020 at 05:18:43PM -0400, Jason Baron wrote:
> >>
> >>
> >> On 3/18/20 3:03 PM, Orson Zhai wrote:
> >>> There is the requirement from new Android that kernel image (GKI) and
> >>> kernel modules are supposed to be built at differnet places. Some people
> >>> want to enable dynamic debug for kernel modules only but not for kernel
> >>> image itself with the consideration of binary size increased or more
> >>> memory being used.
> >>>
> >>> By this patch, dynamic debug is divided into core part (the defination of
> >>> functions) and macro replacement part. We can only have the core part to
> >>> be built-in and do not have to activate the debug output from kenrel image.
> >>>
> >>> Signed-off-by: Orson Zhai <orson.unisoc@gmail.com>
> >>
> >> Hi Orson,
> >>
> >> I think this is a nice feature. Is the idea then that driver can do
> >> something like:
> >>
> >> #if defined(CONFIG_DRIVER_FOO_DEBUG)
> >> #define driver_foo_debug(fmt, ...) \
> >>         dynamic_pr_debug(fmt, ##__VA_ARGS__)
> >> #else
> >>      no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> >> #enif
> >>
> >> And then the Kconfig:
> >>
> >> config DYNAMIC_DRIVER_FOO_DEBUG
> >>      bool "Enable dynamic driver foo printk() support"
> >>      select DYNAMIC_DEBUG_CORE
> >>
> > I highly appreciate you for giving this good example to us.
> > To be honest I did not really think of this kind of usage. :)
> > But it makes much sense. I think dynamic debug might be a little
> > bit high for requirement of memory. Every line of pr_debug will be
> > added with a static data structure and malloc with an item in link table.
> > It might be sensitive especially in embeded system.
> > So this example shows how to avoid to turn on dynamci debug for whole
> > system but part of it when being needed.
> >
> >>
> >> Or did you have something else in mind? Do you have an example
> >> code for the drivers that you mention?
> >
> > My motivation comes from new Andorid GKI release flow. Android kernel team will
> > be in charge of GKI release. And SoC vendors will build their device driver as
> > kernel modules which are diffrent from each vendor. End-users will get their phones
> > installed with GKI plus some modules all together.
> >
> > So at Google side, they can only set DYNAMIC_DEBUG_CORE in their defconfig to build
> > out GKI without worrying about the kernel image size increased too much. Actually
> > GKI is relatively stable as a common binary and there is no strong reason to do
> > dynamic debugging to it.
> >
> > And at vendor side, they will use a local defconfig which is same with Google one but add
> > CONFIG_DYNAMIC_DEBUG to build their kenrel modules. As DYNAMIC_DEBUG enables only a
> > set of macro expansion, so it has no impact to kernel ABI or the modversion.
> > All modules will be compatible with GKI and with dynamic debug enabled.
> >
> > Then the result will be that Google has his clean GKI and vendors have their dynamic-debug-powered modules.
> >
>
>
> static int __init dynamic_debug_init(void)
> {
>         struct _ddebug *iter, *iter_start;
>         const char *modname = NULL;
>         char *cmdline;
>         int ret = 0;
>         int n = 0, entries = 0, modct = 0;
>         int verbose_bytes = 0;
>
>         if (__start___verbose == __stop___verbose) {
>                 pr_warn("_ddebug table is empty in a CONFIG_DYNAMIC_DEBUG build\n");
>                 return 1;

Oh, I forgot this.
If return error here, "ddebug_init_success = 1;" will be never
executed and there will be no debugfs
or /proc operation interface for user.

>         }
>
> ...
>
> I wonder if we should just remove it now.

I think we could keep it by adding "... &&
IS_ENABLED(CONFIG_DYNAMIC_DEBUG)" into the condition.
Then do the comparison again to __start_verbose and __stop_verbose.
If no entries we set ddebug_init_success = 1 and return immediately.

I will make patch V2 if you agree with this.

Best,
-Orson

>
> Thanks,
>
> -Jason
>

Re: [RFC PATCH] dynamic_debug: Add config option of DYNAMIC_DEBUG_CORE

[Orson Zhai]

Hi Jason,

On Thu, Mar 19, 2020 at 5:18 AM Jason Baron <jbaron@akamai.com> wrote:
>
>
>
> On 3/18/20 3:03 PM, Orson Zhai wrote:
> > There is the requirement from new Android that kernel image (GKI) and
> > kernel modules are supposed to be built at differnet places. Some people
> > want to enable dynamic debug for kernel modules only but not for kernel
> > image itself with the consideration of binary size increased or more
> > memory being used.
> >
> > By this patch, dynamic debug is divided into core part (the defination of
> > functions) and macro replacement part. We can only have the core part to
> > be built-in and do not have to activate the debug output from kenrel image.
> >
> > Signed-off-by: Orson Zhai <orson.unisoc@gmail.com>
>
> Hi Orson,
>
> I think this is a nice feature. Is the idea then that driver can do
> something like:
>
> #if defined(CONFIG_DRIVER_FOO_DEBUG)
> #define driver_foo_debug(fmt, ...) \
>         dynamic_pr_debug(fmt, ##__VA_ARGS__)
> #else
>         no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> #enif

I am thinking this again.
Why don't we add a local macro ( I mean not like the global macro from
Kconfig ) just like the DEBUG macro?
Say DDEBUG or DYN_DEBUG.

We can add this to printk.h to let module owners decide when & which
module should be dynamic debugged?
#ifdef CONFIG_DYNAMIC_DUBUG || DDEUG
.....

Module owner can add this to his module's Makefile, for example:
....
ccflags-$(CONFIG_UFS_DEBUG)    += -DDDEBUG
....
This will enable macro replacement for even a single c file.

How do you think about it?

Best,
-Orson

>
> And then the Kconfig:
>
> config DYNAMIC_DRIVER_FOO_DEBUG
>         bool "Enable dynamic driver foo printk() support"
>         select DYNAMIC_DEBUG_CORE
>
>
> Or did you have something else in mind? Do you have an example
> code for the drivers that you mention?
>
> Thanks,
>
> -Jason
>
>
> > ---
> >  include/linux/dynamic_debug.h |  2 +-
> >  lib/Kconfig.debug             | 18 ++++++++++++++++--
> >  lib/Makefile                  |  2 +-
> >  3 files changed, 18 insertions(+), 4 deletions(-)
> >
> > 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/lib/Kconfig.debug b/lib/Kconfig.debug
> > index 69def4a..78a7256 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -97,8 +97,7 @@ config BOOT_PRINTK_DELAY
> >  config DYNAMIC_DEBUG
> >       bool "Enable dynamic printk() support"
> >       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 +163,21 @@ 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.
> > +
> > +       This option is selected when you want to enable dynamic debug
> > +       for kernel modules only but not for the kernel base. Especailly
> > +       in the case that kernel modules are built out of the place where
> > +       kernel base is built.
> > +
> >  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
> >