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

[Nathan Lynch <ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>]

Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org> writes:

> Nathan Lynch wrote:
>> 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.
>
> 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 ?

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.