* [PATCH] dynamic_debug: allow to work if debugfs is disabled @ 2020-01-22 7:43 Greg Kroah-Hartman 2020-01-22 8:03 ` Will Deacon 2020-01-25 0:03 ` kbuild test robot 0 siblings, 2 replies; 27+ messages in thread From: Greg Kroah-Hartman @ 2020-01-22 7:43 UTC (permalink / raw) To: Jason Baron; +Cc: linux-kernel, kernel-team With the realization that having debugfs enabled on "production" systems is generally not a good idea, debugfs is being disabled from more and more platforms over time. However, the functionality of dynamic debugging still is needed at times, and since it relies on debugfs for its user api, having debugfs disabled also forces dynamic debug to be disabled. To get around this, move the "control" file for dynamic_debug to procfs IFF debugfs is disabled. This lets people turn on debugging as needed at runtime for individual driverfs and subsystems. Reported-by: many different companies Cc: Jason Baron <jbaron@akamai.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- .../admin-guide/dynamic-debug-howto.rst | 3 +++ lib/Kconfig.debug | 2 +- lib/dynamic_debug.c | 17 ++++++++++++++--- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst index 252e5ef324e5..41f43a373a6a 100644 --- a/Documentation/admin-guide/dynamic-debug-howto.rst +++ b/Documentation/admin-guide/dynamic-debug-howto.rst @@ -54,6 +54,9 @@ If you make a mistake with the syntax, the write will fail thus:: <debugfs>/dynamic_debug/control -bash: echo: write error: Invalid argument +Note, for systems without 'debugfs' enabled, the control file can be +also found in ``/proc/dynamic_debug/control``. + Viewing Dynamic Debug Behaviour =============================== diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 5ffe144c9794..01d4add8b963 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -98,7 +98,7 @@ config DYNAMIC_DEBUG bool "Enable dynamic printk() support" default n depends on PRINTK - depends on DEBUG_FS + depends on (DEBUG_FS || PROC_FS) help Compiles debug level messages into the kernel, which would not diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index c60409138e13..077b2d6623ac 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c @@ -993,13 +993,24 @@ static __initdata int ddebug_init_success; static int __init dynamic_debug_init_debugfs(void) { - struct dentry *dir; + struct dentry *debugfs_dir; + struct proc_dir_entry *procfs_dir; if (!ddebug_init_success) return -ENODEV; - dir = debugfs_create_dir("dynamic_debug", NULL); - debugfs_create_file("control", 0644, dir, NULL, &ddebug_proc_fops); + /* Create the control file in debugfs if it is enabled */ + if (debugfs_initialized) { + debugfs_dir = debugfs_create_dir("dynamic_debug", NULL); + debugfs_create_file("control", 0644, debugfs_dir, NULL, + &ddebug_proc_fops); + return 0; + } + + /* No debugfs so put it in procfs instead */ + procfs_dir = proc_mkdir("dynamic_debug", NULL); + if (procfs_dir) + proc_create("control", 0x644, procfs_dir, &ddebug_proc_fops); return 0; } -- 2.25.0 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH] dynamic_debug: allow to work if debugfs is disabled 2020-01-22 7:43 [PATCH] dynamic_debug: allow to work if debugfs is disabled Greg Kroah-Hartman @ 2020-01-22 8:03 ` Will Deacon 2020-01-22 8:12 ` Greg Kroah-Hartman 2020-01-25 0:03 ` kbuild test robot 1 sibling, 1 reply; 27+ messages in thread From: Will Deacon @ 2020-01-22 8:03 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: Jason Baron, linux-kernel, kernel-team On Wed, Jan 22, 2020 at 08:43:43AM +0100, Greg Kroah-Hartman wrote: > With the realization that having debugfs enabled on "production" systems is > generally not a good idea, debugfs is being disabled from more and more > platforms over time. However, the functionality of dynamic debugging still is > needed at times, and since it relies on debugfs for its user api, having > debugfs disabled also forces dynamic debug to be disabled. Why is the dyndbg= command-line option not sufficient for these use-cases? > To get around this, move the "control" file for dynamic_debug to procfs IFF > debugfs is disabled. This lets people turn on debugging as needed at runtime > for individual driverfs and subsystems. Hmm. If something called "dynamic_debug" is getting moved out of debugfs, this does raise the question as to what (if anything) should be left behind. I worry this is a bit of a slippery slope... Anywho, comments below. > Reported-by: many different companies > Cc: Jason Baron <jbaron@akamai.com> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > --- > .../admin-guide/dynamic-debug-howto.rst | 3 +++ > lib/Kconfig.debug | 2 +- > lib/dynamic_debug.c | 17 ++++++++++++++--- > 3 files changed, 18 insertions(+), 4 deletions(-) > > diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst > index 252e5ef324e5..41f43a373a6a 100644 > --- a/Documentation/admin-guide/dynamic-debug-howto.rst > +++ b/Documentation/admin-guide/dynamic-debug-howto.rst > @@ -54,6 +54,9 @@ If you make a mistake with the syntax, the write will fail thus:: > <debugfs>/dynamic_debug/control > -bash: echo: write error: Invalid argument > > +Note, for systems without 'debugfs' enabled, the control file can be > +also found in ``/proc/dynamic_debug/control``. > + > Viewing Dynamic Debug Behaviour > =============================== > > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index 5ffe144c9794..01d4add8b963 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -98,7 +98,7 @@ config DYNAMIC_DEBUG > bool "Enable dynamic printk() support" > default n > depends on PRINTK > - depends on DEBUG_FS > + depends on (DEBUG_FS || PROC_FS) > help > > Compiles debug level messages into the kernel, which would noti The help text here also needs updating, since it refers to debugfs. > diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c > index c60409138e13..077b2d6623ac 100644 > --- a/lib/dynamic_debug.c > +++ b/lib/dynamic_debug.c > @@ -993,13 +993,24 @@ static __initdata int ddebug_init_success; > > static int __init dynamic_debug_init_debugfs(void) > { > - struct dentry *dir; > + struct dentry *debugfs_dir; > + struct proc_dir_entry *procfs_dir; > > if (!ddebug_init_success) > return -ENODEV; > > - dir = debugfs_create_dir("dynamic_debug", NULL); > - debugfs_create_file("control", 0644, dir, NULL, &ddebug_proc_fops); > + /* Create the control file in debugfs if it is enabled */ > + if (debugfs_initialized) { > + debugfs_dir = debugfs_create_dir("dynamic_debug", NULL); > + debugfs_create_file("control", 0644, debugfs_dir, NULL, > + &ddebug_proc_fops); > + return 0; > + } > + > + /* No debugfs so put it in procfs instead */ > + procfs_dir = proc_mkdir("dynamic_debug", NULL); > + if (procfs_dir) > + proc_create("control", 0x644, procfs_dir, &ddebug_proc_fops); Shouldn't this be octal rather than hex? Even then, I don't understand what use it is being able to read but not write to this file. Perhaps make it 0600 for /proc ? Will ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] dynamic_debug: allow to work if debugfs is disabled 2020-01-22 8:03 ` Will Deacon @ 2020-01-22 8:12 ` Greg Kroah-Hartman 2020-01-22 13:53 ` [PATCH v2] " Greg Kroah-Hartman 0 siblings, 1 reply; 27+ messages in thread From: Greg Kroah-Hartman @ 2020-01-22 8:12 UTC (permalink / raw) To: Will Deacon; +Cc: Jason Baron, linux-kernel, kernel-team On Wed, Jan 22, 2020 at 08:03:53AM +0000, Will Deacon wrote: > On Wed, Jan 22, 2020 at 08:43:43AM +0100, Greg Kroah-Hartman wrote: > > With the realization that having debugfs enabled on "production" systems is > > generally not a good idea, debugfs is being disabled from more and more > > platforms over time. However, the functionality of dynamic debugging still is > > needed at times, and since it relies on debugfs for its user api, having > > debugfs disabled also forces dynamic debug to be disabled. > > Why is the dyndbg= command-line option not sufficient for these use-cases? They want to enable things after booting, and changing the kernel command line is not something you can do on many systems (i.e. locked-down-bootloaders like embedded systems). Also, the whole option is prevented to be booted if debugfs is not enabled, so the command line wouldn't even work in that situation :) > > To get around this, move the "control" file for dynamic_debug to procfs IFF > > debugfs is disabled. This lets people turn on debugging as needed at runtime > > for individual driverfs and subsystems. > > Hmm. If something called "dynamic_debug" is getting moved out of debugfs, > this does raise the question as to what (if anything) should be left behind. > I worry this is a bit of a slippery slope... I totally agree, but dynamic_debug is independant of debugfs with the exception of the control file itself. > > Reported-by: many different companies > > Cc: Jason Baron <jbaron@akamai.com> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > --- > > .../admin-guide/dynamic-debug-howto.rst | 3 +++ > > lib/Kconfig.debug | 2 +- > > lib/dynamic_debug.c | 17 ++++++++++++++--- > > 3 files changed, 18 insertions(+), 4 deletions(-) > > > > diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst > > index 252e5ef324e5..41f43a373a6a 100644 > > --- a/Documentation/admin-guide/dynamic-debug-howto.rst > > +++ b/Documentation/admin-guide/dynamic-debug-howto.rst > > @@ -54,6 +54,9 @@ If you make a mistake with the syntax, the write will fail thus:: > > <debugfs>/dynamic_debug/control > > -bash: echo: write error: Invalid argument > > > > +Note, for systems without 'debugfs' enabled, the control file can be > > +also found in ``/proc/dynamic_debug/control``. > > + > > Viewing Dynamic Debug Behaviour > > =============================== > > > > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > > index 5ffe144c9794..01d4add8b963 100644 > > --- a/lib/Kconfig.debug > > +++ b/lib/Kconfig.debug > > @@ -98,7 +98,7 @@ config DYNAMIC_DEBUG > > bool "Enable dynamic printk() support" > > default n > > depends on PRINTK > > - depends on DEBUG_FS > > + depends on (DEBUG_FS || PROC_FS) > > help > > > > Compiles debug level messages into the kernel, which would noti > > The help text here also needs updating, since it refers to debugfs. Oops, missed that, thanks! > > diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c > > index c60409138e13..077b2d6623ac 100644 > > --- a/lib/dynamic_debug.c > > +++ b/lib/dynamic_debug.c > > @@ -993,13 +993,24 @@ static __initdata int ddebug_init_success; > > > > static int __init dynamic_debug_init_debugfs(void) > > { > > - struct dentry *dir; > > + struct dentry *debugfs_dir; > > + struct proc_dir_entry *procfs_dir; > > > > if (!ddebug_init_success) > > return -ENODEV; > > > > - dir = debugfs_create_dir("dynamic_debug", NULL); > > - debugfs_create_file("control", 0644, dir, NULL, &ddebug_proc_fops); > > + /* Create the control file in debugfs if it is enabled */ > > + if (debugfs_initialized) { > > + debugfs_dir = debugfs_create_dir("dynamic_debug", NULL); > > + debugfs_create_file("control", 0644, debugfs_dir, NULL, > > + &ddebug_proc_fops); > > + return 0; > > + } > > + > > + /* No debugfs so put it in procfs instead */ > > + procfs_dir = proc_mkdir("dynamic_debug", NULL); > > + if (procfs_dir) > > + proc_create("control", 0x644, procfs_dir, &ddebug_proc_fops); > > Shouldn't this be octal rather than hex? Even then, I don't understand what > use it is being able to read but not write to this file. Perhaps make it > 0600 for /proc ? Argh, my fault, fingers are used to typing hex. You can read from the file to see what the current settings are, I was just trying to mirror the debugfs permissions. thanks, greg k-h ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2] dynamic_debug: allow to work if debugfs is disabled 2020-01-22 8:12 ` Greg Kroah-Hartman @ 2020-01-22 13:53 ` Greg Kroah-Hartman 2020-01-22 18:56 ` Jason Baron 0 siblings, 1 reply; 27+ messages in thread From: Greg Kroah-Hartman @ 2020-01-22 13:53 UTC (permalink / raw) To: Jason Baron; +Cc: Will Deacon, linux-kernel, kernel-team With the realization that having debugfs enabled on "production" systems is generally not a good idea, debugfs is being disabled from more and more platforms over time. However, the functionality of dynamic debugging still is needed at times, and since it relies on debugfs for its user api, having debugfs disabled also forces dynamic debug to be disabled. To get around this, move the "control" file for dynamic_debug to procfs IFF debugfs is disabled. This lets people turn on debugging as needed at runtime for individual driverfs and subsystems. Reported-by: many different companies Cc: Jason Baron <jbaron@akamai.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- v2: Fix up octal permissions and add procfs reference to the Kconfig entry, thanks to Will for the review. .../admin-guide/dynamic-debug-howto.rst | 3 +++ lib/Kconfig.debug | 7 ++++--- lib/dynamic_debug.c | 17 ++++++++++++++--- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst index 252e5ef324e5..41f43a373a6a 100644 --- a/Documentation/admin-guide/dynamic-debug-howto.rst +++ b/Documentation/admin-guide/dynamic-debug-howto.rst @@ -54,6 +54,9 @@ If you make a mistake with the syntax, the write will fail thus:: <debugfs>/dynamic_debug/control -bash: echo: write error: Invalid argument +Note, for systems without 'debugfs' enabled, the control file can be +also found in ``/proc/dynamic_debug/control``. + Viewing Dynamic Debug Behaviour =============================== diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 5ffe144c9794..49980eb8c18e 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -98,7 +98,7 @@ config DYNAMIC_DEBUG bool "Enable dynamic printk() support" default n depends on PRINTK - depends on DEBUG_FS + depends on (DEBUG_FS || PROC_FS) help Compiles debug level messages into the kernel, which would not @@ -116,8 +116,9 @@ config DYNAMIC_DEBUG Usage: Dynamic debugging is controlled via the 'dynamic_debug/control' file, - which is contained in the 'debugfs' filesystem. Thus, the debugfs - filesystem must first be mounted before making use of this feature. + which is contained in the 'debugfs' filesystem or procfs if + debugfs is not present. Thus, the debugfs or procfs filesystem + must first be mounted before making use of this feature. We refer the control file as: <debugfs>/dynamic_debug/control. This file contains a list of the debug statements that can be enabled. The format for each line of the file is: diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index c60409138e13..0f1b26f10fb2 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c @@ -993,13 +993,24 @@ static __initdata int ddebug_init_success; static int __init dynamic_debug_init_debugfs(void) { - struct dentry *dir; + struct dentry *debugfs_dir; + struct proc_dir_entry *procfs_dir; if (!ddebug_init_success) return -ENODEV; - dir = debugfs_create_dir("dynamic_debug", NULL); - debugfs_create_file("control", 0644, dir, NULL, &ddebug_proc_fops); + /* Create the control file in debugfs if it is enabled */ + if (debugfs_initialized) { + debugfs_dir = debugfs_create_dir("dynamic_debug", NULL); + debugfs_create_file("control", 0644, debugfs_dir, NULL, + &ddebug_proc_fops); + return 0; + } + + /* No debugfs so put it in procfs instead */ + procfs_dir = proc_mkdir("dynamic_debug", NULL); + if (procfs_dir) + proc_create("control", 0644, procfs_dir, &ddebug_proc_fops); return 0; } -- 2.25.0 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v2] dynamic_debug: allow to work if debugfs is disabled 2020-01-22 13:53 ` [PATCH v2] " Greg Kroah-Hartman @ 2020-01-22 18:56 ` Jason Baron 2020-01-22 19:29 ` Greg Kroah-Hartman 0 siblings, 1 reply; 27+ messages in thread From: Jason Baron @ 2020-01-22 18:56 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: Will Deacon, linux-kernel, kernel-team On 1/22/20 8:53 AM, Greg Kroah-Hartman wrote: > > With the realization that having debugfs enabled on "production" systems is > generally not a good idea, debugfs is being disabled from more and more > platforms over time. However, the functionality of dynamic debugging still is > needed at times, and since it relies on debugfs for its user api, having > debugfs disabled also forces dynamic debug to be disabled. > > To get around this, move the "control" file for dynamic_debug to procfs IFF > debugfs is disabled. This lets people turn on debugging as needed at runtime > for individual driverfs and subsystems. > Hi Greg, Thanks for updating this. Just a comment below. > Reported-by: many different companies > Cc: Jason Baron <jbaron@akamai.com> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > --- > v2: Fix up octal permissions and add procfs reference to the Kconfig > entry, thanks to Will for the review. > > .../admin-guide/dynamic-debug-howto.rst | 3 +++ > lib/Kconfig.debug | 7 ++++--- > lib/dynamic_debug.c | 17 ++++++++++++++--- > 3 files changed, 21 insertions(+), 6 deletions(-) > > diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst > index 252e5ef324e5..41f43a373a6a 100644 > --- a/Documentation/admin-guide/dynamic-debug-howto.rst > +++ b/Documentation/admin-guide/dynamic-debug-howto.rst > @@ -54,6 +54,9 @@ If you make a mistake with the syntax, the write will fail thus:: > <debugfs>/dynamic_debug/control > -bash: echo: write error: Invalid argument > > +Note, for systems without 'debugfs' enabled, the control file can be > +also found in ``/proc/dynamic_debug/control``. > + > Viewing Dynamic Debug Behaviour > =============================== > > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index 5ffe144c9794..49980eb8c18e 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -98,7 +98,7 @@ config DYNAMIC_DEBUG > bool "Enable dynamic printk() support" > default n > depends on PRINTK > - depends on DEBUG_FS > + depends on (DEBUG_FS || PROC_FS) > help > > Compiles debug level messages into the kernel, which would not > @@ -116,8 +116,9 @@ config DYNAMIC_DEBUG > Usage: > > Dynamic debugging is controlled via the 'dynamic_debug/control' file, > - which is contained in the 'debugfs' filesystem. Thus, the debugfs > - filesystem must first be mounted before making use of this feature. > + which is contained in the 'debugfs' filesystem or procfs if > + debugfs is not present. Thus, the debugfs or procfs filesystem > + must first be mounted before making use of this feature. > We refer the control file as: <debugfs>/dynamic_debug/control. This > file contains a list of the debug statements that can be enabled. The > format for each line of the file is: > diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c > index c60409138e13..0f1b26f10fb2 100644 > --- a/lib/dynamic_debug.c > +++ b/lib/dynamic_debug.c > @@ -993,13 +993,24 @@ static __initdata int ddebug_init_success; > > static int __init dynamic_debug_init_debugfs(void) > { The naming now is a little confusing - dynamic_debug_init_control ? > - struct dentry *dir; > + struct dentry *debugfs_dir; > + struct proc_dir_entry *procfs_dir; > > if (!ddebug_init_success) > return -ENODEV; > > - dir = debugfs_create_dir("dynamic_debug", NULL); > - debugfs_create_file("control", 0644, dir, NULL, &ddebug_proc_fops); > + /* Create the control file in debugfs if it is enabled */ > + if (debugfs_initialized) { > + debugfs_dir = debugfs_create_dir("dynamic_debug", NULL); > + debugfs_create_file("control", 0644, debugfs_dir, NULL, > + &ddebug_proc_fops); > + return 0; > + } > + > + /* No debugfs so put it in procfs instead */ > + procfs_dir = proc_mkdir("dynamic_debug", NULL); > + if (procfs_dir) > + proc_create("control", 0644, procfs_dir, &ddebug_proc_fops); > > return 0; > } > ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2] dynamic_debug: allow to work if debugfs is disabled 2020-01-22 18:56 ` Jason Baron @ 2020-01-22 19:29 ` Greg Kroah-Hartman 2020-01-22 19:31 ` [PATCH v3] " Greg Kroah-Hartman 0 siblings, 1 reply; 27+ messages in thread From: Greg Kroah-Hartman @ 2020-01-22 19:29 UTC (permalink / raw) To: Jason Baron; +Cc: Will Deacon, linux-kernel, kernel-team On Wed, Jan 22, 2020 at 01:56:48PM -0500, Jason Baron wrote: > > > On 1/22/20 8:53 AM, Greg Kroah-Hartman wrote: > > > > With the realization that having debugfs enabled on "production" systems is > > generally not a good idea, debugfs is being disabled from more and more > > platforms over time. However, the functionality of dynamic debugging still is > > needed at times, and since it relies on debugfs for its user api, having > > debugfs disabled also forces dynamic debug to be disabled. > > > > To get around this, move the "control" file for dynamic_debug to procfs IFF > > debugfs is disabled. This lets people turn on debugging as needed at runtime > > for individual driverfs and subsystems. > > > > Hi Greg, > > Thanks for updating this. Just a comment below. > > > > Reported-by: many different companies > > Cc: Jason Baron <jbaron@akamai.com> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > --- > > v2: Fix up octal permissions and add procfs reference to the Kconfig > > entry, thanks to Will for the review. > > > > .../admin-guide/dynamic-debug-howto.rst | 3 +++ > > lib/Kconfig.debug | 7 ++++--- > > lib/dynamic_debug.c | 17 ++++++++++++++--- > > 3 files changed, 21 insertions(+), 6 deletions(-) > > > > diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst > > index 252e5ef324e5..41f43a373a6a 100644 > > --- a/Documentation/admin-guide/dynamic-debug-howto.rst > > +++ b/Documentation/admin-guide/dynamic-debug-howto.rst > > @@ -54,6 +54,9 @@ If you make a mistake with the syntax, the write will fail thus:: > > <debugfs>/dynamic_debug/control > > -bash: echo: write error: Invalid argument > > > > +Note, for systems without 'debugfs' enabled, the control file can be > > +also found in ``/proc/dynamic_debug/control``. > > + > > Viewing Dynamic Debug Behaviour > > =============================== > > > > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > > index 5ffe144c9794..49980eb8c18e 100644 > > --- a/lib/Kconfig.debug > > +++ b/lib/Kconfig.debug > > @@ -98,7 +98,7 @@ config DYNAMIC_DEBUG > > bool "Enable dynamic printk() support" > > default n > > depends on PRINTK > > - depends on DEBUG_FS > > + depends on (DEBUG_FS || PROC_FS) > > help > > > > Compiles debug level messages into the kernel, which would not > > @@ -116,8 +116,9 @@ config DYNAMIC_DEBUG > > Usage: > > > > Dynamic debugging is controlled via the 'dynamic_debug/control' file, > > - which is contained in the 'debugfs' filesystem. Thus, the debugfs > > - filesystem must first be mounted before making use of this feature. > > + which is contained in the 'debugfs' filesystem or procfs if > > + debugfs is not present. Thus, the debugfs or procfs filesystem > > + must first be mounted before making use of this feature. > > We refer the control file as: <debugfs>/dynamic_debug/control. This > > file contains a list of the debug statements that can be enabled. The > > format for each line of the file is: > > diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c > > index c60409138e13..0f1b26f10fb2 100644 > > --- a/lib/dynamic_debug.c > > +++ b/lib/dynamic_debug.c > > @@ -993,13 +993,24 @@ static __initdata int ddebug_init_success; > > > > static int __init dynamic_debug_init_debugfs(void) > > { > > The naming now is a little confusing - dynamic_debug_init_control ? Ah, good point. I'll go fix that up, and fix up the foolish build warning that I missed... thanks, greg k-h ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v3] dynamic_debug: allow to work if debugfs is disabled 2020-01-22 19:29 ` Greg Kroah-Hartman @ 2020-01-22 19:31 ` Greg Kroah-Hartman 2020-01-22 21:43 ` Randy Dunlap 2020-01-23 15:53 ` [PATCH v3] " Theodore Y. Ts'o 0 siblings, 2 replies; 27+ messages in thread From: Greg Kroah-Hartman @ 2020-01-22 19:31 UTC (permalink / raw) To: Jason Baron; +Cc: Will Deacon, linux-kernel, kernel-team With the realization that having debugfs enabled on "production" systems is generally not a good idea, debugfs is being disabled from more and more platforms over time. However, the functionality of dynamic debugging still is needed at times, and since it relies on debugfs for its user api, having debugfs disabled also forces dynamic debug to be disabled. To get around this, move the "control" file for dynamic_debug to procfs IFF debugfs is disabled. This lets people turn on debugging as needed at runtime for individual driverfs and subsystems. Reported-by: many different companies Cc: Jason Baron <jbaron@akamai.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- v3: rename init function as it is now no longer just for debugfs, thanks to Jason for the review. Fix build warning for debugfs_initialized call. v2: Fix up octal permissions and add procfs reference to the Kconfig entry, thanks to Will for the review. .../admin-guide/dynamic-debug-howto.rst | 3 +++ lib/Kconfig.debug | 7 ++++--- lib/dynamic_debug.c | 21 ++++++++++++++----- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst index 252e5ef324e5..41f43a373a6a 100644 --- a/Documentation/admin-guide/dynamic-debug-howto.rst +++ b/Documentation/admin-guide/dynamic-debug-howto.rst @@ -54,6 +54,9 @@ If you make a mistake with the syntax, the write will fail thus:: <debugfs>/dynamic_debug/control -bash: echo: write error: Invalid argument +Note, for systems without 'debugfs' enabled, the control file can be +also found in ``/proc/dynamic_debug/control``. + Viewing Dynamic Debug Behaviour =============================== diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 5ffe144c9794..49980eb8c18e 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -98,7 +98,7 @@ config DYNAMIC_DEBUG bool "Enable dynamic printk() support" default n depends on PRINTK - depends on DEBUG_FS + depends on (DEBUG_FS || PROC_FS) help Compiles debug level messages into the kernel, which would not @@ -116,8 +116,9 @@ config DYNAMIC_DEBUG Usage: Dynamic debugging is controlled via the 'dynamic_debug/control' file, - which is contained in the 'debugfs' filesystem. Thus, the debugfs - filesystem must first be mounted before making use of this feature. + which is contained in the 'debugfs' filesystem or procfs if + debugfs is not present. Thus, the debugfs or procfs filesystem + must first be mounted before making use of this feature. We refer the control file as: <debugfs>/dynamic_debug/control. This file contains a list of the debug statements that can be enabled. The format for each line of the file is: diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index c60409138e13..118e928cdbda 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c @@ -991,15 +991,26 @@ static void ddebug_remove_all_tables(void) static __initdata int ddebug_init_success; -static int __init dynamic_debug_init_debugfs(void) +static int __init dynamic_debug_init_control(void) { - struct dentry *dir; + struct proc_dir_entry *procfs_dir; + struct dentry *debugfs_dir; if (!ddebug_init_success) return -ENODEV; - dir = debugfs_create_dir("dynamic_debug", NULL); - debugfs_create_file("control", 0644, dir, NULL, &ddebug_proc_fops); + /* Create the control file in debugfs if it is enabled */ + if (debugfs_initialized()) { + debugfs_dir = debugfs_create_dir("dynamic_debug", NULL); + debugfs_create_file("control", 0644, debugfs_dir, NULL, + &ddebug_proc_fops); + return 0; + } + + /* No debugfs so put it in procfs instead */ + procfs_dir = proc_mkdir("dynamic_debug", NULL); + if (procfs_dir) + proc_create("control", 0644, procfs_dir, &ddebug_proc_fops); return 0; } @@ -1077,4 +1088,4 @@ static int __init dynamic_debug_init(void) early_initcall(dynamic_debug_init); /* Debugfs setup must be done later */ -fs_initcall(dynamic_debug_init_debugfs); +fs_initcall(dynamic_debug_init_control); -- 2.25.0 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v3] dynamic_debug: allow to work if debugfs is disabled 2020-01-22 19:31 ` [PATCH v3] " Greg Kroah-Hartman @ 2020-01-22 21:43 ` Randy Dunlap 2020-01-23 8:48 ` Greg Kroah-Hartman 2020-01-23 15:53 ` [PATCH v3] " Theodore Y. Ts'o 1 sibling, 1 reply; 27+ messages in thread From: Randy Dunlap @ 2020-01-22 21:43 UTC (permalink / raw) To: Greg Kroah-Hartman, Jason Baron; +Cc: Will Deacon, linux-kernel, kernel-team Hi Greg, If you make any more changes: On 1/22/20 11:31 AM, Greg Kroah-Hartman wrote: > diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst > index 252e5ef324e5..41f43a373a6a 100644 > --- a/Documentation/admin-guide/dynamic-debug-howto.rst > +++ b/Documentation/admin-guide/dynamic-debug-howto.rst > @@ -54,6 +54,9 @@ If you make a mistake with the syntax, the write will fail thus:: > <debugfs>/dynamic_debug/control > -bash: echo: write error: Invalid argument > > +Note, for systems without 'debugfs' enabled, the control file can be > +also found in ``/proc/dynamic_debug/control``. Mostly drop the "also". How about: > +Note, for systems without 'debugfs' enabled, the control file is located > +in ``/proc/dynamic_debug/control``. > + > Viewing Dynamic Debug Behaviour > =============================== > -- ~Randy ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3] dynamic_debug: allow to work if debugfs is disabled 2020-01-22 21:43 ` Randy Dunlap @ 2020-01-23 8:48 ` Greg Kroah-Hartman 2020-01-23 8:50 ` [PATCH v4] " Greg Kroah-Hartman 0 siblings, 1 reply; 27+ messages in thread From: Greg Kroah-Hartman @ 2020-01-23 8:48 UTC (permalink / raw) To: Randy Dunlap; +Cc: Jason Baron, Will Deacon, linux-kernel, kernel-team On Wed, Jan 22, 2020 at 01:43:46PM -0800, Randy Dunlap wrote: > Hi Greg, > > If you make any more changes: > > On 1/22/20 11:31 AM, Greg Kroah-Hartman wrote: > > diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst > > index 252e5ef324e5..41f43a373a6a 100644 > > --- a/Documentation/admin-guide/dynamic-debug-howto.rst > > +++ b/Documentation/admin-guide/dynamic-debug-howto.rst > > @@ -54,6 +54,9 @@ If you make a mistake with the syntax, the write will fail thus:: > > <debugfs>/dynamic_debug/control > > -bash: echo: write error: Invalid argument > > > > +Note, for systems without 'debugfs' enabled, the control file can be > > +also found in ``/proc/dynamic_debug/control``. > > Mostly drop the "also". How about: > > > +Note, for systems without 'debugfs' enabled, the control file is located > > +in ``/proc/dynamic_debug/control``. Much nicer, I'll respin it with this change, thanks for the review! greg k-h ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v4] dynamic_debug: allow to work if debugfs is disabled 2020-01-23 8:48 ` Greg Kroah-Hartman @ 2020-01-23 8:50 ` Greg Kroah-Hartman 2020-01-23 9:36 ` Will Deacon 0 siblings, 1 reply; 27+ messages in thread From: Greg Kroah-Hartman @ 2020-01-23 8:50 UTC (permalink / raw) To: Jason Baron; +Cc: Randy Dunlap, Will Deacon, linux-kernel, kernel-team With the realization that having debugfs enabled on "production" systems is generally not a good idea, debugfs is being disabled from more and more platforms over time. However, the functionality of dynamic debugging still is needed at times, and since it relies on debugfs for its user api, having debugfs disabled also forces dynamic debug to be disabled. To get around this, move the "control" file for dynamic_debug to procfs IFF debugfs is disabled. This lets people turn on debugging as needed at runtime for individual driverfs and subsystems. Reported-by: many different companies Cc: Jason Baron <jbaron@akamai.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- v4: tweaks to the .rst text thanks to Randy's review v3: rename init function as it is now no longer just for debugfs, thanks to Jason for the review. Fix build warning for debugfs_initialized call. v2: Fix up octal permissions and add procfs reference to the Kconfig entry, thanks to Will for the review. .../admin-guide/dynamic-debug-howto.rst | 3 +++ lib/Kconfig.debug | 7 ++++--- lib/dynamic_debug.c | 21 ++++++++++++++----- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst index 252e5ef324e5..60679dda6007 100644 --- a/Documentation/admin-guide/dynamic-debug-howto.rst +++ b/Documentation/admin-guide/dynamic-debug-howto.rst @@ -54,6 +54,9 @@ If you make a mistake with the syntax, the write will fail thus:: <debugfs>/dynamic_debug/control -bash: echo: write error: Invalid argument +Note, for systems without 'debugfs' enabled, the control file is located +in ``/proc/dynamic_debug/control``. + Viewing Dynamic Debug Behaviour =============================== diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 5ffe144c9794..49980eb8c18e 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -98,7 +98,7 @@ config DYNAMIC_DEBUG bool "Enable dynamic printk() support" default n depends on PRINTK - depends on DEBUG_FS + depends on (DEBUG_FS || PROC_FS) help Compiles debug level messages into the kernel, which would not @@ -116,8 +116,9 @@ config DYNAMIC_DEBUG Usage: Dynamic debugging is controlled via the 'dynamic_debug/control' file, - which is contained in the 'debugfs' filesystem. Thus, the debugfs - filesystem must first be mounted before making use of this feature. + which is contained in the 'debugfs' filesystem or procfs if + debugfs is not present. Thus, the debugfs or procfs filesystem + must first be mounted before making use of this feature. We refer the control file as: <debugfs>/dynamic_debug/control. This file contains a list of the debug statements that can be enabled. The format for each line of the file is: diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index c60409138e13..118e928cdbda 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c @@ -991,15 +991,26 @@ static void ddebug_remove_all_tables(void) static __initdata int ddebug_init_success; -static int __init dynamic_debug_init_debugfs(void) +static int __init dynamic_debug_init_control(void) { - struct dentry *dir; + struct proc_dir_entry *procfs_dir; + struct dentry *debugfs_dir; if (!ddebug_init_success) return -ENODEV; - dir = debugfs_create_dir("dynamic_debug", NULL); - debugfs_create_file("control", 0644, dir, NULL, &ddebug_proc_fops); + /* Create the control file in debugfs if it is enabled */ + if (debugfs_initialized()) { + debugfs_dir = debugfs_create_dir("dynamic_debug", NULL); + debugfs_create_file("control", 0644, debugfs_dir, NULL, + &ddebug_proc_fops); + return 0; + } + + /* No debugfs so put it in procfs instead */ + procfs_dir = proc_mkdir("dynamic_debug", NULL); + if (procfs_dir) + proc_create("control", 0644, procfs_dir, &ddebug_proc_fops); return 0; } @@ -1077,4 +1088,4 @@ static int __init dynamic_debug_init(void) early_initcall(dynamic_debug_init); /* Debugfs setup must be done later */ -fs_initcall(dynamic_debug_init_debugfs); +fs_initcall(dynamic_debug_init_control); -- 2.25.0 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v4] dynamic_debug: allow to work if debugfs is disabled 2020-01-23 8:50 ` [PATCH v4] " Greg Kroah-Hartman @ 2020-01-23 9:36 ` Will Deacon 0 siblings, 0 replies; 27+ messages in thread From: Will Deacon @ 2020-01-23 9:36 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: Jason Baron, Randy Dunlap, linux-kernel, kernel-team On Thu, Jan 23, 2020 at 09:50:15AM +0100, Greg Kroah-Hartman wrote: > With the realization that having debugfs enabled on "production" systems is > generally not a good idea, debugfs is being disabled from more and more > platforms over time. However, the functionality of dynamic debugging still is > needed at times, and since it relies on debugfs for its user api, having > debugfs disabled also forces dynamic debug to be disabled. > > To get around this, move the "control" file for dynamic_debug to procfs IFF > debugfs is disabled. This lets people turn on debugging as needed at runtime > for individual driverfs and subsystems. > > Reported-by: many different companies > Cc: Jason Baron <jbaron@akamai.com> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > --- > v4: tweaks to the .rst text thanks to Randy's review > v3: rename init function as it is now no longer just for debugfs, thanks > to Jason for the review. > Fix build warning for debugfs_initialized call. > v2: Fix up octal permissions and add procfs reference to the Kconfig > entry, thanks to Will for the review. > > .../admin-guide/dynamic-debug-howto.rst | 3 +++ > lib/Kconfig.debug | 7 ++++--- > lib/dynamic_debug.c | 21 ++++++++++++++----- > 3 files changed, 23 insertions(+), 8 deletions(-) I had a brief "oh crap" moment when I thought you were exposing both the procfs and debugfs interfaces at the same time, but thankfully that's not the case. Whilst it's a bit of a shame that it's come to this, the code looks pretty decent to me, so: Acked-by: Will Deacon <will@kernel.org> Thanks, Will ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3] dynamic_debug: allow to work if debugfs is disabled 2020-01-22 19:31 ` [PATCH v3] " Greg Kroah-Hartman 2020-01-22 21:43 ` Randy Dunlap @ 2020-01-23 15:53 ` Theodore Y. Ts'o 2020-01-23 17:55 ` Greg Kroah-Hartman 2020-02-09 11:05 ` [PATCH v5] " Greg Kroah-Hartman 1 sibling, 2 replies; 27+ messages in thread From: Theodore Y. Ts'o @ 2020-01-23 15:53 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: Jason Baron, Will Deacon, linux-kernel, kernel-team On Wed, Jan 22, 2020 at 08:31:18PM +0100, Greg Kroah-Hartman wrote: > With the realization that having debugfs enabled on "production" systems is > generally not a good idea, debugfs is being disabled from more and more > platforms over time. However, the functionality of dynamic debugging still is > needed at times, and since it relies on debugfs for its user api, having > debugfs disabled also forces dynamic debug to be disabled. > > To get around this, move the "control" file for dynamic_debug to procfs IFF > debugfs is disabled. This lets people turn on debugging as needed at runtime > for individual driverfs and subsystems. Instead of moving the control file IFF debugfs is enabled, what about always making it available in /proc, and marking the control file for dynamic_debug in debugfs as deprecated? It would seem to me that this would cause less confusion in the future.... - Ted ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3] dynamic_debug: allow to work if debugfs is disabled 2020-01-23 15:53 ` [PATCH v3] " Theodore Y. Ts'o @ 2020-01-23 17:55 ` Greg Kroah-Hartman 2020-01-24 6:02 ` Theodore Y. Ts'o 2020-02-09 11:05 ` [PATCH v5] " Greg Kroah-Hartman 1 sibling, 1 reply; 27+ messages in thread From: Greg Kroah-Hartman @ 2020-01-23 17:55 UTC (permalink / raw) To: Theodore Y. Ts'o; +Cc: Jason Baron, Will Deacon, linux-kernel, kernel-team On Thu, Jan 23, 2020 at 10:53:40AM -0500, Theodore Y. Ts'o wrote: > On Wed, Jan 22, 2020 at 08:31:18PM +0100, Greg Kroah-Hartman wrote: > > With the realization that having debugfs enabled on "production" systems is > > generally not a good idea, debugfs is being disabled from more and more > > platforms over time. However, the functionality of dynamic debugging still is > > needed at times, and since it relies on debugfs for its user api, having > > debugfs disabled also forces dynamic debug to be disabled. > > > > To get around this, move the "control" file for dynamic_debug to procfs IFF > > debugfs is disabled. This lets people turn on debugging as needed at runtime > > for individual driverfs and subsystems. > > Instead of moving the control file IFF debugfs is enabled, what about > always making it available in /proc, and marking the control file for > dynamic_debug in debugfs as deprecated? It would seem to me that this > would cause less confusion in the future.... Why deprecate it? It's fine where it is, and most developer's have debugfs enabled so all is good. I'd rather only use /proc as a last-resort. thanks, greg k-h ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3] dynamic_debug: allow to work if debugfs is disabled 2020-01-23 17:55 ` Greg Kroah-Hartman @ 2020-01-24 6:02 ` Theodore Y. Ts'o 2020-01-24 7:29 ` Greg Kroah-Hartman 0 siblings, 1 reply; 27+ messages in thread From: Theodore Y. Ts'o @ 2020-01-24 6:02 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: Jason Baron, Will Deacon, linux-kernel, kernel-team On Thu, Jan 23, 2020 at 06:55:36PM +0100, Greg Kroah-Hartman wrote: > > Instead of moving the control file IFF debugfs is enabled, what about > > always making it available in /proc, and marking the control file for > > dynamic_debug in debugfs as deprecated? It would seem to me that this > > would cause less confusion in the future.... > > Why deprecate it? It's fine where it is, and most developer's have > debugfs enabled so all is good. I'd rather only use /proc as a > last-resort. This makes life difficult for scripts that manipulate the control file, since they now need to check two different locations -- either /sys/kernel/debug or /proc. It's likely that people who normally use distribution kernels where debugfs is disabled will have scripts which are hard-coded to look in /proc, and then when they build a kernel with debugfs enabled, the /proc entry will go **poof**, and their script will break. So regardless of what we do with the control file in debugfs, it might be nice if moving forward, scripts can count on the /proc file existing. Cheers, - Ted ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3] dynamic_debug: allow to work if debugfs is disabled 2020-01-24 6:02 ` Theodore Y. Ts'o @ 2020-01-24 7:29 ` Greg Kroah-Hartman 2020-01-25 1:42 ` Theodore Y. Ts'o 0 siblings, 1 reply; 27+ messages in thread From: Greg Kroah-Hartman @ 2020-01-24 7:29 UTC (permalink / raw) To: Theodore Y. Ts'o; +Cc: Jason Baron, Will Deacon, linux-kernel, kernel-team On Fri, Jan 24, 2020 at 01:02:00AM -0500, Theodore Y. Ts'o wrote: > On Thu, Jan 23, 2020 at 06:55:36PM +0100, Greg Kroah-Hartman wrote: > > > Instead of moving the control file IFF debugfs is enabled, what about > > > always making it available in /proc, and marking the control file for > > > dynamic_debug in debugfs as deprecated? It would seem to me that this > > > would cause less confusion in the future.... > > > > Why deprecate it? It's fine where it is, and most developer's have > > debugfs enabled so all is good. I'd rather only use /proc as a > > last-resort. > > This makes life difficult for scripts that manipulate the control > file, since they now need to check two different locations -- either > /sys/kernel/debug or /proc. That is literally 2 extra lines in a script file. If you point me at any that actually used the existing control file, I will be glad to fix them up :) > It's likely that people who normally use > distribution kernels where debugfs is disabled will have scripts which > are hard-coded to look in /proc, and then when they build a kernel > with debugfs enabled, the /proc entry will go **poof**, and their > script will break. **poof** they didn't test it :) Seriously, I am doing this change to make it _easier_ for those people who want debugfs disabled, yet want this type of debugging. This is much better than having no debugging at all. Don't put extra complexity in the kernel for when it can be trivially handled in userspace, you know this :) thanks, greg k-h ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3] dynamic_debug: allow to work if debugfs is disabled 2020-01-24 7:29 ` Greg Kroah-Hartman @ 2020-01-25 1:42 ` Theodore Y. Ts'o 2020-01-25 17:11 ` Jonathan Corbet 0 siblings, 1 reply; 27+ messages in thread From: Theodore Y. Ts'o @ 2020-01-25 1:42 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: Jason Baron, Will Deacon, linux-kernel, kernel-team On Fri, Jan 24, 2020 at 08:29:40AM +0100, Greg Kroah-Hartman wrote: > > It's likely that people who normally use > > distribution kernels where debugfs is disabled will have scripts which > > are hard-coded to look in /proc, and then when they build a kernel > > with debugfs enabled, the /proc entry will go **poof**, and their > > script will break. > > **poof** they didn't test it :) > > Seriously, I am doing this change to make it _easier_ for those people > who want debugfs disabled, yet want this type of debugging. This is > much better than having no debugging at all. > > Don't put extra complexity in the kernel for when it can be trivially > handled in userspace, you know this :) It's also trivial to handle this in the kernel by potentially having the control file appear in two places. Consider what it would be like to explain this in the Linux documentation --- "the control file could be here, or it could be there", depending on how the kernel is configured. The complexity of documenting this, and the complexity in userspace; and we have to have the code in the source code for the file to be in the appear in both places *anyway*. We've done a lot more to maintain userspace backwards compatibility that Linus demands; and while I recognize this is not exactly the same thing, being consistent about where to find the control file would be even *easier* for userspace programmers. And is it really that hard to provide this consistency in the kernel? Cheers, - Ted ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3] dynamic_debug: allow to work if debugfs is disabled 2020-01-25 1:42 ` Theodore Y. Ts'o @ 2020-01-25 17:11 ` Jonathan Corbet 2020-01-27 22:19 ` Saravana Kannan 0 siblings, 1 reply; 27+ messages in thread From: Jonathan Corbet @ 2020-01-25 17:11 UTC (permalink / raw) To: Theodore Y. Ts'o Cc: Greg Kroah-Hartman, Jason Baron, Will Deacon, linux-kernel, kernel-team On Fri, 24 Jan 2020 20:42:31 -0500 "Theodore Y. Ts'o" <tytso@mit.edu> wrote: > On Fri, Jan 24, 2020 at 08:29:40AM +0100, Greg Kroah-Hartman wrote: > > > It's likely that people who normally use > > > distribution kernels where debugfs is disabled will have scripts which > > > are hard-coded to look in /proc, and then when they build a kernel > > > with debugfs enabled, the /proc entry will go **poof**, and their > > > script will break. > > > > **poof** they didn't test it :) > > > > Seriously, I am doing this change to make it _easier_ for those people > > who want debugfs disabled, yet want this type of debugging. This is > > much better than having no debugging at all. > > > > Don't put extra complexity in the kernel for when it can be trivially > > handled in userspace, you know this :) > > It's also trivial to handle this in the kernel by potentially having > the control file appear in two places. Consider what it would be like > to explain this in the Linux documentation --- "the control file could > be here, or it could be there", depending on how the kernel is > configured. The complexity of documenting this, and the complexity in > userspace; and we have to have the code in the source code for the > file to be in the appear in both places *anyway*. FWIW, avoiding the need to document something like this has been a motivating factor behind a number of my patches over the years. Moving a control knob based on kernel configuration is a user-hostile feature. Scripts can be made to cope with this kind of behavior in one place; if you let such things accumulate in a system, though, it gets to the point where it's really hard for anybody to keep track of all the pieces and be sure that their code will work. If dynamic debug is meant to be a feature supported on all kernels, it should, IMO, be lifted out of debugfs and made into a proper feature - with a compatibility link left behind in debugfs for as long as it's needed, of course. </sermon> :) jon ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3] dynamic_debug: allow to work if debugfs is disabled 2020-01-25 17:11 ` Jonathan Corbet @ 2020-01-27 22:19 ` Saravana Kannan 0 siblings, 0 replies; 27+ messages in thread From: Saravana Kannan @ 2020-01-27 22:19 UTC (permalink / raw) To: Jonathan Corbet Cc: Theodore Y. Ts'o, Greg Kroah-Hartman, Jason Baron, Will Deacon, LKML, Android Kernel Team On Sat, Jan 25, 2020 at 9:11 AM Jonathan Corbet <corbet@lwn.net> wrote: > > On Fri, 24 Jan 2020 20:42:31 -0500 > "Theodore Y. Ts'o" <tytso@mit.edu> wrote: > > > On Fri, Jan 24, 2020 at 08:29:40AM +0100, Greg Kroah-Hartman wrote: > > > > It's likely that people who normally use > > > > distribution kernels where debugfs is disabled will have scripts which > > > > are hard-coded to look in /proc, and then when they build a kernel > > > > with debugfs enabled, the /proc entry will go **poof**, and their > > > > script will break. > > > > > > **poof** they didn't test it :) > > > > > > Seriously, I am doing this change to make it _easier_ for those people > > > who want debugfs disabled, yet want this type of debugging. This is > > > much better than having no debugging at all. > > > > > > Don't put extra complexity in the kernel for when it can be trivially > > > handled in userspace, you know this :) > > > > It's also trivial to handle this in the kernel by potentially having > > the control file appear in two places. Consider what it would be like > > to explain this in the Linux documentation --- "the control file could > > be here, or it could be there", depending on how the kernel is > > configured. The complexity of documenting this, and the complexity in > > userspace; and we have to have the code in the source code for the > > file to be in the appear in both places *anyway*. > > FWIW, avoiding the need to document something like this has been a > motivating factor behind a number of my patches over the years. > > Moving a control knob based on kernel configuration is a user-hostile > feature. Scripts can be made to cope with this kind of behavior in one > place; if you let such things accumulate in a system, though, it gets to > the point where it's really hard for anybody to keep track of all the > pieces and be sure that their code will work. > > If dynamic debug is meant to be a feature supported on all kernels, it > should, IMO, be lifted out of debugfs and made into a proper feature - with > a compatibility link left behind in debugfs for as long as it's needed, of > course. > > </sermon> :) My 2 cents -- agree with what Ted and Jon have said. -Saravana ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5] dynamic_debug: allow to work if debugfs is disabled 2020-01-23 15:53 ` [PATCH v3] " Theodore Y. Ts'o 2020-01-23 17:55 ` Greg Kroah-Hartman @ 2020-02-09 11:05 ` Greg Kroah-Hartman 2020-02-09 15:53 ` Joe Perches 2020-02-10 21:11 ` [PATCH v6] " Greg Kroah-Hartman 1 sibling, 2 replies; 27+ messages in thread From: Greg Kroah-Hartman @ 2020-02-09 11:05 UTC (permalink / raw) To: Theodore Y. Ts'o, Jonathan Corbet, Saravana Kannan, Jason Baron, Will Deacon Cc: linux-kernel, kernel-team, Randy Dunlap With the realization that having debugfs enabled on "production" systems is generally not a good idea, debugfs is being disabled from more and more platforms over time. However, the functionality of dynamic debugging still is needed at times, and since it relies on debugfs for its user api, having debugfs disabled also forces dynamic debug to be disabled. To get around this, also create the "control" file for dynamic_debug in procfs. This allows people turn on debugging as needed at runtime for individual driverfs and subsystems. Reported-by: many different companies Cc: Jason Baron <jbaron@akamai.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- v5: as many people asked for it, now enable the control file in both debugfs and procfs at the same time. v4: tweaks to the .rst text thanks to Randy's review v3: rename init function as it is now no longer just for debugfs, thanks to Jason for the review. Fix build warning for debugfs_initialized call. v2: Fix up octal permissions and add procfs reference to the Kconfig entry, thanks to Will for the review. .../admin-guide/dynamic-debug-howto.rst | 3 +++ lib/Kconfig.debug | 7 ++++--- lib/dynamic_debug.c | 20 ++++++++++++++----- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst index 252e5ef324e5..585451d12608 100644 --- a/Documentation/admin-guide/dynamic-debug-howto.rst +++ b/Documentation/admin-guide/dynamic-debug-howto.rst @@ -54,6 +54,9 @@ If you make a mistake with the syntax, the write will fail thus:: <debugfs>/dynamic_debug/control -bash: echo: write error: Invalid argument +Note, for systems without 'debugfs' enabled, the control file can also +be found in ``/proc/dynamic_debug/control``. + Viewing Dynamic Debug Behaviour =============================== diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 69def4a9df00..a15dde66dc4c 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -98,7 +98,7 @@ config DYNAMIC_DEBUG bool "Enable dynamic printk() support" default n depends on PRINTK - depends on DEBUG_FS + depends on (DEBUG_FS || PROC_FS) help Compiles debug level messages into the kernel, which would not @@ -116,8 +116,9 @@ config DYNAMIC_DEBUG Usage: Dynamic debugging is controlled via the 'dynamic_debug/control' file, - which is contained in the 'debugfs' filesystem. Thus, the debugfs - filesystem must first be mounted before making use of this feature. + which is contained in the 'debugfs' filesystem or procfs if + debugfs is not present. Thus, the debugfs or procfs filesystem + must first be mounted before making use of this feature. We refer the control file as: <debugfs>/dynamic_debug/control. This file contains a list of the debug statements that can be enabled. The format for each line of the file is: diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index c60409138e13..c220c1891729 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c @@ -991,15 +991,25 @@ static void ddebug_remove_all_tables(void) static __initdata int ddebug_init_success; -static int __init dynamic_debug_init_debugfs(void) +static int __init dynamic_debug_init_control(void) { - struct dentry *dir; + struct proc_dir_entry *procfs_dir; + struct dentry *debugfs_dir; if (!ddebug_init_success) return -ENODEV; - dir = debugfs_create_dir("dynamic_debug", NULL); - debugfs_create_file("control", 0644, dir, NULL, &ddebug_proc_fops); + /* Create the control file in debugfs if it is enabled */ + if (debugfs_initialized()) { + debugfs_dir = debugfs_create_dir("dynamic_debug", NULL); + debugfs_create_file("control", 0644, debugfs_dir, NULL, + &ddebug_proc_fops); + } + + /* Also create the control file in procfs */ + procfs_dir = proc_mkdir("dynamic_debug", NULL); + if (procfs_dir) + proc_create("control", 0644, procfs_dir, &ddebug_proc_fops); return 0; } @@ -1077,4 +1087,4 @@ static int __init dynamic_debug_init(void) early_initcall(dynamic_debug_init); /* Debugfs setup must be done later */ -fs_initcall(dynamic_debug_init_debugfs); +fs_initcall(dynamic_debug_init_control); -- 2.25.0 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v5] dynamic_debug: allow to work if debugfs is disabled 2020-02-09 11:05 ` [PATCH v5] " Greg Kroah-Hartman @ 2020-02-09 15:53 ` Joe Perches 2020-02-09 17:03 ` Greg Kroah-Hartman 2020-02-10 21:11 ` [PATCH v6] " Greg Kroah-Hartman 1 sibling, 1 reply; 27+ messages in thread From: Joe Perches @ 2020-02-09 15:53 UTC (permalink / raw) To: Greg Kroah-Hartman, Theodore Y. Ts'o, Jonathan Corbet, Saravana Kannan, Jason Baron, Will Deacon Cc: linux-kernel, kernel-team, Randy Dunlap On Sun, 2020-02-09 at 12:05 +0100, Greg Kroah-Hartman wrote: > With the realization that having debugfs enabled on "production" systems > is generally not a good idea, debugfs is being disabled from more and > more platforms over time. However, the functionality of dynamic > debugging still is needed at times, and since it relies on debugfs for > its user api, having debugfs disabled also forces dynamic debug to be > disabled. > > To get around this, also create the "control" file for dynamic_debug in > procfs. This allows people turn on debugging as needed at runtime for > individual driverfs and subsystems. > > Reported-by: many different companies > Cc: Jason Baron <jbaron@akamai.com> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > --- > v5: as many people asked for it, now enable the control file in both > debugfs and procfs at the same time. So now there can be differences in the two control files and these are readable files are sometimes parsed by scripts. It'd be better to figure out how to soft link the files. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v5] dynamic_debug: allow to work if debugfs is disabled 2020-02-09 15:53 ` Joe Perches @ 2020-02-09 17:03 ` Greg Kroah-Hartman 0 siblings, 0 replies; 27+ messages in thread From: Greg Kroah-Hartman @ 2020-02-09 17:03 UTC (permalink / raw) To: Joe Perches Cc: Theodore Y. Ts'o, Jonathan Corbet, Saravana Kannan, Jason Baron, Will Deacon, linux-kernel, kernel-team, Randy Dunlap On Sun, Feb 09, 2020 at 07:53:38AM -0800, Joe Perches wrote: > On Sun, 2020-02-09 at 12:05 +0100, Greg Kroah-Hartman wrote: > > With the realization that having debugfs enabled on "production" systems > > is generally not a good idea, debugfs is being disabled from more and > > more platforms over time. However, the functionality of dynamic > > debugging still is needed at times, and since it relies on debugfs for > > its user api, having debugfs disabled also forces dynamic debug to be > > disabled. > > > > To get around this, also create the "control" file for dynamic_debug in > > procfs. This allows people turn on debugging as needed at runtime for > > individual driverfs and subsystems. > > > > Reported-by: many different companies > > Cc: Jason Baron <jbaron@akamai.com> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > --- > > v5: as many people asked for it, now enable the control file in both > > debugfs and procfs at the same time. > > So now there can be differences in the two control files > and these are readable files are sometimes parsed by > scripts. What difference will there be? They should both be the same, as they point to the identical fops behind the virtual file. > It'd be better to figure out how to soft link the files. A symlink is not going to work, but this should be fine from what I can tell. I'll do some more debugging tonight, but all was fine last I tried this last week. thanks, greg k-h ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v6] dynamic_debug: allow to work if debugfs is disabled 2020-02-09 11:05 ` [PATCH v5] " Greg Kroah-Hartman 2020-02-09 15:53 ` Joe Perches @ 2020-02-10 21:11 ` Greg Kroah-Hartman 2020-02-10 21:15 ` Randy Dunlap 2020-02-11 11:01 ` Will Deacon 1 sibling, 2 replies; 27+ messages in thread From: Greg Kroah-Hartman @ 2020-02-10 21:11 UTC (permalink / raw) To: Theodore Y. Ts'o, Jonathan Corbet, Saravana Kannan, Jason Baron, Will Deacon Cc: linux-kernel, kernel-team, Randy Dunlap With the realization that having debugfs enabled on "production" systems is generally not a good idea, debugfs is being disabled from more and more platforms over time. However, the functionality of dynamic debugging still is needed at times, and since it relies on debugfs for its user api, having debugfs disabled also forces dynamic debug to be disabled. To get around this, also create the "control" file for dynamic_debug in procfs. This allows people turn on debugging as needed at runtime for individual driverfs and subsystems. Reported-by: many different companies Cc: Jason Baron <jbaron@akamai.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- v6: fix up Kconfig help, it was a bit incorrect,thanks to Saravana for the review. v5: as many people asked for it, now enable the control file in both debugfs and procfs at the same time. v4: tweaks to the .rst text thanks to Randy's review v3: rename init function as it is now no longer just for debugfs, thanks to Jason for the review. Fix build warning for debugfs_initialized call. v2: Fix up octal permissions and add procfs reference to the Kconfig entry, thanks to Will for the review. .../admin-guide/dynamic-debug-howto.rst | 3 +++ lib/Kconfig.debug | 7 ++++--- lib/dynamic_debug.c | 20 ++++++++++++++----- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst index 252e5ef324e5..585451d12608 100644 --- a/Documentation/admin-guide/dynamic-debug-howto.rst +++ b/Documentation/admin-guide/dynamic-debug-howto.rst @@ -54,6 +54,9 @@ If you make a mistake with the syntax, the write will fail thus:: <debugfs>/dynamic_debug/control -bash: echo: write error: Invalid argument +Note, for systems without 'debugfs' enabled, the control file can also +be found in ``/proc/dynamic_debug/control``. + Viewing Dynamic Debug Behaviour =============================== diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 69def4a9df00..7f4992fd8a2e 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -98,7 +98,7 @@ config DYNAMIC_DEBUG bool "Enable dynamic printk() support" default n depends on PRINTK - depends on DEBUG_FS + depends on (DEBUG_FS || PROC_FS) help Compiles debug level messages into the kernel, which would not @@ -116,8 +116,9 @@ config DYNAMIC_DEBUG Usage: Dynamic debugging is controlled via the 'dynamic_debug/control' file, - which is contained in the 'debugfs' filesystem. Thus, the debugfs - filesystem must first be mounted before making use of this feature. + which is contained in the 'debugfs' filesystem or procfs. + Thus, the debugfs or procfs filesystem must first be mounted before + making use of this feature. We refer the control file as: <debugfs>/dynamic_debug/control. This file contains a list of the debug statements that can be enabled. The format for each line of the file is: diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index c60409138e13..c220c1891729 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c @@ -991,15 +991,25 @@ static void ddebug_remove_all_tables(void) static __initdata int ddebug_init_success; -static int __init dynamic_debug_init_debugfs(void) +static int __init dynamic_debug_init_control(void) { - struct dentry *dir; + struct proc_dir_entry *procfs_dir; + struct dentry *debugfs_dir; if (!ddebug_init_success) return -ENODEV; - dir = debugfs_create_dir("dynamic_debug", NULL); - debugfs_create_file("control", 0644, dir, NULL, &ddebug_proc_fops); + /* Create the control file in debugfs if it is enabled */ + if (debugfs_initialized()) { + debugfs_dir = debugfs_create_dir("dynamic_debug", NULL); + debugfs_create_file("control", 0644, debugfs_dir, NULL, + &ddebug_proc_fops); + } + + /* Also create the control file in procfs */ + procfs_dir = proc_mkdir("dynamic_debug", NULL); + if (procfs_dir) + proc_create("control", 0644, procfs_dir, &ddebug_proc_fops); return 0; } @@ -1077,4 +1087,4 @@ static int __init dynamic_debug_init(void) early_initcall(dynamic_debug_init); /* Debugfs setup must be done later */ -fs_initcall(dynamic_debug_init_debugfs); +fs_initcall(dynamic_debug_init_control); base-commit: bb6d3fb354c5ee8d6bde2d576eb7220ea09862b9 -- 2.25.0 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v6] dynamic_debug: allow to work if debugfs is disabled 2020-02-10 21:11 ` [PATCH v6] " Greg Kroah-Hartman @ 2020-02-10 21:15 ` Randy Dunlap 2020-02-12 21:58 ` Greg Kroah-Hartman 2020-02-11 11:01 ` Will Deacon 1 sibling, 1 reply; 27+ messages in thread From: Randy Dunlap @ 2020-02-10 21:15 UTC (permalink / raw) To: Greg Kroah-Hartman, Theodore Y. Ts'o, Jonathan Corbet, Saravana Kannan, Jason Baron, Will Deacon Cc: linux-kernel, kernel-team On 2/10/20 1:11 PM, Greg Kroah-Hartman wrote: > diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst > index 252e5ef324e5..585451d12608 100644 > --- a/Documentation/admin-guide/dynamic-debug-howto.rst > +++ b/Documentation/admin-guide/dynamic-debug-howto.rst > @@ -54,6 +54,9 @@ If you make a mistake with the syntax, the write will fail thus:: > <debugfs>/dynamic_debug/control > -bash: echo: write error: Invalid argument > > +Note, for systems without 'debugfs' enabled, the control file can also Hi Greg, If you make any more changes, please drop the word "also" here ^^^^ > +be found in ``/proc/dynamic_debug/control``. > + > Viewing Dynamic Debug Behaviour > =============================== > -- ~Randy ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v6] dynamic_debug: allow to work if debugfs is disabled 2020-02-10 21:15 ` Randy Dunlap @ 2020-02-12 21:58 ` Greg Kroah-Hartman 0 siblings, 0 replies; 27+ messages in thread From: Greg Kroah-Hartman @ 2020-02-12 21:58 UTC (permalink / raw) To: Randy Dunlap Cc: Theodore Y. Ts'o, Jonathan Corbet, Saravana Kannan, Jason Baron, Will Deacon, linux-kernel, kernel-team On Mon, Feb 10, 2020 at 01:15:50PM -0800, Randy Dunlap wrote: > On 2/10/20 1:11 PM, Greg Kroah-Hartman wrote: > > diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst > > index 252e5ef324e5..585451d12608 100644 > > --- a/Documentation/admin-guide/dynamic-debug-howto.rst > > +++ b/Documentation/admin-guide/dynamic-debug-howto.rst > > @@ -54,6 +54,9 @@ If you make a mistake with the syntax, the write will fail thus:: > > <debugfs>/dynamic_debug/control > > -bash: echo: write error: Invalid argument > > > > +Note, for systems without 'debugfs' enabled, the control file can also > > Hi Greg, > If you make any more changes, please drop the word "also" here ^^^^ I will delete it by hand when I apply the patch now, thanks! greg k-h ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v6] dynamic_debug: allow to work if debugfs is disabled 2020-02-10 21:11 ` [PATCH v6] " Greg Kroah-Hartman 2020-02-10 21:15 ` Randy Dunlap @ 2020-02-11 11:01 ` Will Deacon 1 sibling, 0 replies; 27+ messages in thread From: Will Deacon @ 2020-02-11 11:01 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Theodore Y. Ts'o, Jonathan Corbet, Saravana Kannan, Jason Baron, linux-kernel, kernel-team, Randy Dunlap On Mon, Feb 10, 2020 at 01:11:42PM -0800, Greg Kroah-Hartman wrote: > With the realization that having debugfs enabled on "production" systems > is generally not a good idea, debugfs is being disabled from more and > more platforms over time. However, the functionality of dynamic > debugging still is needed at times, and since it relies on debugfs for > its user api, having debugfs disabled also forces dynamic debug to be > disabled. > > To get around this, also create the "control" file for dynamic_debug in > procfs. This allows people turn on debugging as needed at runtime for > individual driverfs and subsystems. > > Reported-by: many different companies > Cc: Jason Baron <jbaron@akamai.com> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > --- > v6: fix up Kconfig help, it was a bit incorrect,thanks to Saravana for > the review. > v5: as many people asked for it, now enable the control file in both > debugfs and procfs at the same time. The 'ddebug_lock' mutex looks like it resolves all of the races here, so: Acked-by: Will Deacon <will@kernel.org> Will ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] dynamic_debug: allow to work if debugfs is disabled 2020-01-22 7:43 [PATCH] dynamic_debug: allow to work if debugfs is disabled Greg Kroah-Hartman @ 2020-01-25 0:03 ` kbuild test robot 2020-01-25 0:03 ` kbuild test robot 1 sibling, 0 replies; 27+ messages in thread From: kbuild test robot @ 2020-01-25 0:03 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: kbuild-all, Jason Baron, linux-kernel, kernel-team [-- Attachment #1: Type: text/plain, Size: 2342 bytes --] Hi Greg, I love your patch! Perhaps something to improve: [auto build test WARNING on linux/master] [also build test WARNING on linus/master v5.5-rc7] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Greg-Kroah-Hartman/dynamic_debug-allow-to-work-if-debugfs-is-disabled/20200124-140304 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 1522d9da40bdfe502c91163e6d769332897201fa config: m68k-allmodconfig (attached as .config) compiler: m68k-linux-gcc (GCC) 7.5.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.5.0 make.cross ARCH=m68k If you fix the issue, kindly add following tag Reported-by: kbuild test robot <lkp@intel.com> All warnings (new ones prefixed by >>): lib/dynamic_debug.c: In function 'dynamic_debug_init_debugfs': >> lib/dynamic_debug.c:1003:6: warning: the address of 'debugfs_initialized' will always evaluate as 'true' [-Waddress] if (debugfs_initialized) { ^~~~~~~~~~~~~~~~~~~ vim +1003 lib/dynamic_debug.c 993 994 static int __init dynamic_debug_init_debugfs(void) 995 { 996 struct dentry *debugfs_dir; 997 struct proc_dir_entry *procfs_dir; 998 999 if (!ddebug_init_success) 1000 return -ENODEV; 1001 1002 /* Create the control file in debugfs if it is enabled */ > 1003 if (debugfs_initialized) { 1004 debugfs_dir = debugfs_create_dir("dynamic_debug", NULL); 1005 debugfs_create_file("control", 0644, debugfs_dir, NULL, 1006 &ddebug_proc_fops); 1007 return 0; 1008 } 1009 1010 /* No debugfs so put it in procfs instead */ 1011 procfs_dir = proc_mkdir("dynamic_debug", NULL); 1012 if (procfs_dir) 1013 proc_create("control", 0x644, procfs_dir, &ddebug_proc_fops); 1014 1015 return 0; 1016 } 1017 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 51852 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] dynamic_debug: allow to work if debugfs is disabled @ 2020-01-25 0:03 ` kbuild test robot 0 siblings, 0 replies; 27+ messages in thread From: kbuild test robot @ 2020-01-25 0:03 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 2405 bytes --] Hi Greg, I love your patch! Perhaps something to improve: [auto build test WARNING on linux/master] [also build test WARNING on linus/master v5.5-rc7] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Greg-Kroah-Hartman/dynamic_debug-allow-to-work-if-debugfs-is-disabled/20200124-140304 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 1522d9da40bdfe502c91163e6d769332897201fa config: m68k-allmodconfig (attached as .config) compiler: m68k-linux-gcc (GCC) 7.5.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.5.0 make.cross ARCH=m68k If you fix the issue, kindly add following tag Reported-by: kbuild test robot <lkp@intel.com> All warnings (new ones prefixed by >>): lib/dynamic_debug.c: In function 'dynamic_debug_init_debugfs': >> lib/dynamic_debug.c:1003:6: warning: the address of 'debugfs_initialized' will always evaluate as 'true' [-Waddress] if (debugfs_initialized) { ^~~~~~~~~~~~~~~~~~~ vim +1003 lib/dynamic_debug.c 993 994 static int __init dynamic_debug_init_debugfs(void) 995 { 996 struct dentry *debugfs_dir; 997 struct proc_dir_entry *procfs_dir; 998 999 if (!ddebug_init_success) 1000 return -ENODEV; 1001 1002 /* Create the control file in debugfs if it is enabled */ > 1003 if (debugfs_initialized) { 1004 debugfs_dir = debugfs_create_dir("dynamic_debug", NULL); 1005 debugfs_create_file("control", 0644, debugfs_dir, NULL, 1006 &ddebug_proc_fops); 1007 return 0; 1008 } 1009 1010 /* No debugfs so put it in procfs instead */ 1011 procfs_dir = proc_mkdir("dynamic_debug", NULL); 1012 if (procfs_dir) 1013 proc_create("control", 0x644, procfs_dir, &ddebug_proc_fops); 1014 1015 return 0; 1016 } 1017 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org Intel Corporation [-- Attachment #2: config.gz --] [-- Type: application/gzip, Size: 51852 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2020-02-12 21:58 UTC | newest] Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-01-22 7:43 [PATCH] dynamic_debug: allow to work if debugfs is disabled Greg Kroah-Hartman 2020-01-22 8:03 ` Will Deacon 2020-01-22 8:12 ` Greg Kroah-Hartman 2020-01-22 13:53 ` [PATCH v2] " Greg Kroah-Hartman 2020-01-22 18:56 ` Jason Baron 2020-01-22 19:29 ` Greg Kroah-Hartman 2020-01-22 19:31 ` [PATCH v3] " Greg Kroah-Hartman 2020-01-22 21:43 ` Randy Dunlap 2020-01-23 8:48 ` Greg Kroah-Hartman 2020-01-23 8:50 ` [PATCH v4] " Greg Kroah-Hartman 2020-01-23 9:36 ` Will Deacon 2020-01-23 15:53 ` [PATCH v3] " Theodore Y. Ts'o 2020-01-23 17:55 ` Greg Kroah-Hartman 2020-01-24 6:02 ` Theodore Y. Ts'o 2020-01-24 7:29 ` Greg Kroah-Hartman 2020-01-25 1:42 ` Theodore Y. Ts'o 2020-01-25 17:11 ` Jonathan Corbet 2020-01-27 22:19 ` Saravana Kannan 2020-02-09 11:05 ` [PATCH v5] " Greg Kroah-Hartman 2020-02-09 15:53 ` Joe Perches 2020-02-09 17:03 ` Greg Kroah-Hartman 2020-02-10 21:11 ` [PATCH v6] " Greg Kroah-Hartman 2020-02-10 21:15 ` Randy Dunlap 2020-02-12 21:58 ` Greg Kroah-Hartman 2020-02-11 11:01 ` Will Deacon 2020-01-25 0:03 ` [PATCH] " kbuild test robot 2020-01-25 0:03 ` kbuild test robot
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.