C/R: simplify checkpoint debugging
PATCH 1/1 C/R: simplify checkpoint debugging
To enable printing of checkpoint/restart messages, one must enable CHECKPOINT_DEBUG and boot with ckpt_debug= ... something I haven't been able to figure out. Further complicating matters is that if DYNAMIC_DEBUG is enabled, you still won't get any debugging output unless you manually enable debug output for all the functions you're interested in via <debugfs>/dynamic_debug/control. At this stage of development most users will want all C/R messages either on or off and don't need a fancy filtering mechanism.
Change ckpt_debug to print messages if CHECKPOINT_DEBUG=y. Otherwise it will emit nothing. Remove ckpt_debug= setup code. Reword CHECKPOINT_DEBUG config help text appropriately.
--- checkpoint/sys.c | 22 ---------------------- include/linux/checkpoint.h | 4 +--- lib/Kconfig.debug | 9 +++------ 3 files changed, 4 insertions(+), 31 deletions(-)
checkpoint/sys.c +0 -22
@@ -338,25 +338,3 @@ SYSCALL_DEFINE3(restart, pid_t, pid, int, fd, unsigned long, flags)ckpt_ctx_put (ctx);return ret; } - - -/* 'ckpt_debug_level' controls the verbosity level of c/r code */ -#ifdef CONFIG_CHECKPOINT_DEBUG - -/* FIX: allow to change during runtime */ -unsigned long __read_mostly ckpt_debug_level =CKPT_DDEFAULT ; - -static __init int ckpt_debug_setup (char *s) -{ -long val, ret; - - ret =strict_strtoul (s,10 , &val); -if (ret <0 ) -return ret; - ckpt_debug_level = val; -return 0 ; -} - -__setup ("ckpt_debug=" , ckpt_debug_setup); - -#endif /* CONFIG_CHECKPOINT_DEBUG */
include/linux/checkpoint.h +1 -3
@@ -264,13 +264,11 @@ extern int restore_memory_contents(struct ckpt_ctx *ctx, struct inode *inode);#endif #ifdef CONFIG_CHECKPOINT_DEBUG -extern unsigned long ckpt_debug_level;/* use this to select a specific debug level */ #define _ckpt_debug(level, fmt, args...) \ do { \ - if (ckpt_debug_level & (level)) \ - pr_debug("[%d:c/r:%s] " fmt, \ + printk(KERN_DEBUG "[%d:c/r:%s] " fmt, \ task_pid_vnr(current), \ __func__, ## args); \ } while (0)
lib/Kconfig.debug +3 -6
@@ -966,13 +966,10 @@ config CHECKPOINT_DEBUG depends on CHECKPOINT default y help - This options turns on the debugging output of checkpoint/restart. - The level of verbosity is controlled by 'ckpt_debug_level' and can - be set at boot time with "ckpt_debug=" option. + If enabled, debugging messages related to checkpoint and + restart operations will be printed to the kernel log. - Turning this option off will reduce the size of the c/r code. If - turned on, it is unlikely to incur visible overhead if the debug - level is set to zero. + Turn this on if you plan to test checkpoint/restart. source "samples/Kconfig"
Signature · 1 line
1.6.0.6
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 9 lines · Nathan Lynch
To enable printing of checkpoint/restart messages, one must enable
CHECKPOINT_DEBUGand boot with ckpt_debug= ... something I haven't been able to figure out. Further complicating matters is that ifDYNAMIC_DEBUGis enabled, you still won't get any debugging output unless you manually enable debug output for all the functions you're interested in via <debugfs>/dynamic_debug/control. At this stage of development most users will want all C/R messages either on or off and don't need a fancy filtering mechanism.
Quoted 5 lines
Change
ckpt_debugto print messages if CHECKPOINT_DEBUG=y. Otherwise it will emit nothing. Remove ckpt_debug= setup code. RewordCHECKPOINT_DEBUGconfig help text appropriately.Signed-off-by Nathan Lynch <[email protected]>
Yes please!
-serge
Quoted 83 lines
--- checkpoint/sys.c | 22 ---------------------- include/linux/checkpoint.h | 4 +--- lib/Kconfig.debug | 9 +++------ 3 files changed, 4 insertions(+), 31 deletions(-)checkpoint/sys.c +0
-22@@ -338,25 +338,3 @@ SYSCALL_DEFINE3(restart, pid_t, pid, int, fd, unsigned long, flags)ckpt_ctx_put (ctx);return ret; } - - -/* 'ckpt_debug_level' controls the verbosity level of c/r code */ -#ifdef CONFIG_CHECKPOINT_DEBUG - -/* FIX: allow to change during runtime */ -unsigned long __read_mostly ckpt_debug_level =CKPT_DDEFAULT ; - -static __init int ckpt_debug_setup (char *s) -{ -long val, ret; - - ret =strict_strtoul (s,10 , &val); -if (ret <0 ) -return ret; - ckpt_debug_level = val; -return 0 ; -} - -__setup ("ckpt_debug=" , ckpt_debug_setup); - -#endif /* CONFIG_CHECKPOINT_DEBUG */ include/linux/checkpoint.h +1
-3@@ -264,13 +264,11 @@ extern int restore_memory_contents(struct ckpt_ctx *ctx, struct inode *inode);#endif #ifdef CONFIG_CHECKPOINT_DEBUG -extern unsigned long ckpt_debug_level;/* use this to select a specific debug level */ #define _ckpt_debug(level, fmt, args...) \ do { \ - if (ckpt_debug_level & (level)) \ - pr_debug("[%d:c/r:%s] " fmt, \ + printk(KERN_DEBUG "[%d:c/r:%s] " fmt, \ task_pid_vnr(current), \ __func__, ## args); \ } while (0)lib/Kconfig.debug +3
-6@@ -966,13 +966,10 @@ config CHECKPOINT_DEBUG depends on CHECKPOINT default y help - This options turns on the debugging output of checkpoint/restart. - The level of verbosity is controlled by 'ckpt_debug_level' and can - be set at boot time with "ckpt_debug=" option. + If enabled, debugging messages related to checkpoint and + restart operations will be printed to the kernel log. - Turning this option off will reduce the size of the c/r code. If - turned on, it is unlikely to incur visible overhead if the debug - level is set to zero. + Turn this on if you plan to test checkpoint/restart. source "samples/Kconfig"Signature · 6 lines
1.6.0.6 _______________________________________________ Containers mailing list Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org https://lists.linux-foundation.org/mailman/listinfo/containers
A message that is not in the archive (sent off-list or not yet received): 20090622204059.GB5537-r/[email protected]
SH> Quoting Nathan Lynch ([email protected]):
Quoted 9 lines
Quoted 9 lines
To enable printing of checkpoint/restart messages, one must enable
CHECKPOINT_DEBUGand boot with ckpt_debug= ... something I haven't been able to figure out. Further complicating matters is that ifDYNAMIC_DEBUGis enabled, you still won't get any debugging output unless you manually enable debug output for all the functions you're interested in via <debugfs>/dynamic_debug/control. At this stage of development most users will want all C/R messages either on or off and don't need a fancy filtering mechanism.
Quoted 5 lines
Quoted 5 lines
Change
ckpt_debugto print messages if CHECKPOINT_DEBUG=y. Otherwise it will emit nothing. Remove ckpt_debug= setup code. RewordCHECKPOINT_DEBUGconfig help text appropriately.Signed-off-by Nathan Lynch <[email protected]>
SH> Yes please!
Enthusiastically seconded. Or thirded, or whatever it would be.
Signature · 3 lines
Dan Smith IBM Linux Technology Center email: danms-r/[email protected]
Quoted 8 lines · Nathan Lynch
To enable printing of checkpoint/restart messages, one must enable
CHECKPOINT_DEBUGand boot with ckpt_debug= ... something I haven't been able to figure out. Further complicating matters is that ifDYNAMIC_DEBUGis enabled, you still won't get any debugging output unless you manually enable debug output for all the functions you're interested in via <debugfs>/dynamic_debug/control. At this stage of development most users will want all C/R messages either on or off and don't need a fancy filtering mechanism.
Thanks for bringing this up. In part it has been annoying to me too.
I'm all for replacing pr_debug(...) with printk(KERN_DEBUG ...). Done.
I prefer to keep the selection of debug verbosity in - it becomes useful when debugging a checkpoint or restart that produce plenty of output. It keeps the noise (and time) to an acceptable level.
So I did also change the default (CKPT_DDEFAULT) to select all debug flags. So the normal behavior is as intended in this patch.
Given the enthusiastic responses to the proposed changes, do you still wish to remove the option to select verbosity level ?
Oren.
Quoted 81 lines
Change
ckpt_debugto print messages if CHECKPOINT_DEBUG=y. Otherwise it will emit nothing. Remove ckpt_debug= setup code. RewordCHECKPOINT_DEBUGconfig help text appropriately.Signed-off-by Nathan Lynch <[email protected]>--- checkpoint/sys.c | 22 ---------------------- include/linux/checkpoint.h | 4 +--- lib/Kconfig.debug | 9 +++------ 3 files changed, 4 insertions(+), 31 deletions(-)checkpoint/sys.c +0
-22@@ -338,25 +338,3 @@ SYSCALL_DEFINE3(restart, pid_t, pid, int, fd, unsigned long, flags)ckpt_ctx_put (ctx);return ret; } - - -/* 'ckpt_debug_level' controls the verbosity level of c/r code */ -#ifdef CONFIG_CHECKPOINT_DEBUG - -/* FIX: allow to change during runtime */ -unsigned long __read_mostly ckpt_debug_level =CKPT_DDEFAULT ; - -static __init int ckpt_debug_setup (char *s) -{ -long val, ret; - - ret =strict_strtoul (s,10 , &val); -if (ret <0 ) -return ret; - ckpt_debug_level = val; -return 0 ; -} - -__setup ("ckpt_debug=" , ckpt_debug_setup); - -#endif /* CONFIG_CHECKPOINT_DEBUG */ include/linux/checkpoint.h +1
-3@@ -264,13 +264,11 @@ extern int restore_memory_contents(struct ckpt_ctx *ctx, struct inode *inode);#endif #ifdef CONFIG_CHECKPOINT_DEBUG -extern unsigned long ckpt_debug_level;/* use this to select a specific debug level */ #define _ckpt_debug(level, fmt, args...) \ do { \ - if (ckpt_debug_level & (level)) \ - pr_debug("[%d:c/r:%s] " fmt, \ + printk(KERN_DEBUG "[%d:c/r:%s] " fmt, \ task_pid_vnr(current), \ __func__, ## args); \ } while (0)lib/Kconfig.debug +3
-6@@ -966,13 +966,10 @@ config CHECKPOINT_DEBUG depends on CHECKPOINT default y help - This options turns on the debugging output of checkpoint/restart. - The level of verbosity is controlled by 'ckpt_debug_level' and can - be set at boot time with "ckpt_debug=" option. + If enabled, debugging messages related to checkpoint and + restart operations will be printed to the kernel log. - Turning this option off will reduce the size of the c/r code. If - turned on, it is unlikely to incur visible overhead if the debug - level is set to zero. + Turn this on if you plan to test checkpoint/restart. source "samples/Kconfig"
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 19 lines · Oren Laadan
Quoted 8 lines · Nathan Lynch
To enable printing of checkpoint/restart messages, one must enable
CHECKPOINT_DEBUGand boot with ckpt_debug= ... something I haven't been able to figure out. Further complicating matters is that ifDYNAMIC_DEBUGis enabled, you still won't get any debugging output unless you manually enable debug output for all the functions you're interested in via <debugfs>/dynamic_debug/control. At this stage of development most users will want all C/R messages either on or off and don't need a fancy filtering mechanism.Thanks for bringing this up. In part it has been annoying to me too.
I'm all for replacing
pr_debug(...)withprintk(KERN_DEBUG ...). Done.I prefer to keep the selection of debug verbosity in - it becomes useful when debugging a checkpoint or restart that produce plenty of output. It keeps the noise (and time) to an acceptable level.
Quoted 5 lines
So I did also change the default (
CKPT_DDEFAULT) to select all debug flags. So the normal behavior is as intended in this patch.Given the enthusiastic responses to the proposed changes, do you still wish to remove the option to select verbosity level ?
If you add some documentation (in comments or kconfig help) on how to use ckpt_debug= I wouldn't object. AFAIK one has to read the code to figure out which values to use. Apologies if I've missed something...
In the long term, however, detailed reporting on C/R operations may be better implemented using static tracepoints -- less runtime overhead than printk, and (I think) they can be selectively enabled, which would satisfy the speed/noise concerns you raise above.