Re: [PATCH] C/R: simplify checkpoint debugging

["Serge E. Hallyn" <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>]

Quoting Nathan Lynch (ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org):
> 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.
> 
> Signed-off-by: Nathan Lynch <ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>

Yes please!

-serge

> ---
>  checkpoint/sys.c           |   22 ----------------------
>  include/linux/checkpoint.h |    4 +---
>  lib/Kconfig.debug          |    9 +++------
>  3 files changed, 4 insertions(+), 31 deletions(-)
> 
> diff --git a/checkpoint/sys.c b/checkpoint/sys.c
> index 38a5299..3c0056f 100644
> --- a/checkpoint/sys.c
> +++ b/checkpoint/sys.c
> @@ -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 */
> diff --git a/include/linux/checkpoint.h b/include/linux/checkpoint.h
> index b92bf95..f1aad34 100644
> --- a/include/linux/checkpoint.h
> +++ b/include/linux/checkpoint.h
> @@ -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)
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index ab795a6..2f6556b 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -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"
> 
> -- 
> 1.6.0.6
> 
> _______________________________________________
> Containers mailing list
> Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linux-foundation.org/mailman/listinfo/containers