Messages in this thread | | | From | Orson Zhai <> | Date | Sat, 21 Mar 2020 02:30:31 +0800 | Subject | Re: [RFC PATCH] dynamic_debug: Add config option of DYNAMIC_DEBUG_CORE |
| |
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 >
| |