RFC: dynamic debug enhancements?

RFC: dynamic debug enhancements?

[Joe Perches]

Hey Jason.

I recently went through drivers/media and updated
lots of calls to pr_<level>.

A common pattern for debugging there and elsewhere
treewide is the use of macros like:

#define dprintk(level, fmt, ...)			\
do {							\
	if (level > [some_modparam_var])		\
		pr_debug(fmt, ##__VA_ARGS__);		\
} while (0)

and

#define dprintk(mask, fmt, ...)				\
do {							\
	if (mask & [some_modparam_var])			\
		pr_debug(fmt, ##__VA_ARGS__);		\
} while (0)

It might be useful to have standardized calls
like pr_debug_level and pr_debug_mask instead
of multiple hand-rolled variants treewide.

Another common thing was the use of various
__FILE__, __func__, __LINE__ outputs.

I think __FILE__ is not particularly useful and
can reasonably be replaced by KBUILD_MODNAME.

Perhaps it would be good to have options to
enable these outputs with specific controls
for dynamic_debug uses.

Maybe something like using a define similar to
pr_fmt for what options are preselected for
various ddebug outputs like:

#define DYNAMIC_DEBUG_DEFAULT_FLAGS 		\
	(_DPRINTK_FLAGS_INCL_MODNAME |		\
	 _DPRINTK_FLAGS_INCL_FUNCNAME |		\
	 _DPRINTK_FLAGS_INCL_LINENO)

Re: RFC: dynamic debug enhancements?

[Jason Baron]

On Sun, Aug 21, 2011 at 05:15:43PM -0700, Joe Perches wrote:
> 
> I recently went through drivers/media and updated
> lots of calls to pr_<level>.
> 
> A common pattern for debugging there and elsewhere
> treewide is the use of macros like:
> 
> #define dprintk(level, fmt, ...)			\
> do {							\
> 	if (level > [some_modparam_var])		\
> 		pr_debug(fmt, ##__VA_ARGS__);		\
> } while (0)
> 
> and
> 
> #define dprintk(mask, fmt, ...)				\
> do {							\
> 	if (mask & [some_modparam_var])			\
> 		pr_debug(fmt, ##__VA_ARGS__);		\
> } while (0)
> 
> It might be useful to have standardized calls
> like pr_debug_level and pr_debug_mask instead
> of multiple hand-rolled variants treewide.
> 
> Another common thing was the use of various
> __FILE__, __func__, __LINE__ outputs.
> 
> I think __FILE__ is not particularly useful and
> can reasonably be replaced by KBUILD_MODNAME.
> 
> Perhaps it would be good to have options to
> enable these outputs with specific controls
> for dynamic_debug uses.
> 
> Maybe something like using a define similar to
> pr_fmt for what options are preselected for
> various ddebug outputs like:
> 
> #define DYNAMIC_DEBUG_DEFAULT_FLAGS 		\
> 	(_DPRINTK_FLAGS_INCL_MODNAME |		\
> 	 _DPRINTK_FLAGS_INCL_FUNCNAME |		\
> 	 _DPRINTK_FLAGS_INCL_LINENO)
> 
> 

Hi Joe,

looks interesting. I'm wondering how we handle module parameters though?
In the dynamic debug disabled case, we'd have to standardize the module
params names. And for the dynamic debug enabled case, I'm not sure how
we would honor those module params?

-Jason

Re: RFC: dynamic debug enhancements?

[Joe Perches]

On Tue, 2011-08-23 at 10:23 -0400, Jason Baron wrote:
> On Sun, Aug 21, 2011 at 05:15:43PM -0700, Joe Perches wrote:
> > I recently went through drivers/media and updated
> > lots of calls to pr_<level>.
> > A common pattern for debugging there and elsewhere
> > treewide is the use of macros like:
> > #define dprintk(level, fmt, ...)			\
> > do {							\
> > 	if (level > [some_modparam_var])		\
> > 		pr_debug(fmt, ##__VA_ARGS__);		\
> > } while (0)
> > and
> > #define dprintk(mask, fmt, ...)				\
> > do {							\
> > 	if (mask & [some_modparam_var])			\
> > 		pr_debug(fmt, ##__VA_ARGS__);		\
> > } while (0)
> > It might be useful to have standardized calls
> > like pr_debug_level and pr_debug_mask instead
> > of multiple hand-rolled variants treewide.
[]
> looks interesting. I'm wondering how we handle module parameters though?
> In the dynamic debug disabled case, we'd have to standardize the module
> params names. And for the dynamic debug enabled case, I'm not sure how
> we would honor those module params?

One possibility would be to require a #define
before use.

Something like the DEBUG_VARIABLE used in
drivers/media/ and include/media/.

Maybe something like:

#define DYNAMIC_DEBUG_LEVEL_VARIABLE module_var_foo
and/or
#define DYNAMIC_DEBUG_MASK_VARIABLE module_var_bar

and

$ echo 'module <foo> mask <n>' > <debugfs>/dynamic_debug/control
and
$ echo 'module <foo> level <n>' > <debugfs>/dynamic_debug/control

could work to set the module control variable too.

Re: RFC: dynamic debug enhancements?

[Jim Cromie]

On Tue, Aug 23, 2011 at 7:00 PM, Joe Perches <joe@perches.com> wrote:
> On Tue, 2011-08-23 at 10:23 -0400, Jason Baron wrote:
>> On Sun, Aug 21, 2011 at 05:15:43PM -0700, Joe Perches wrote:
>> > I recently went through drivers/media and updated
>> > lots of calls to pr_<level>.
>> > A common pattern for debugging there and elsewhere
>> > treewide is the use of macros like:
>> > #define dprintk(level, fmt, ...)                    \
>> > do {                                                        \
>> >     if (level > [some_modparam_var])                \
>> >             pr_debug(fmt, ##__VA_ARGS__);           \
>> > } while (0)
>> > and
>> > #define dprintk(mask, fmt, ...)                             \
>> > do {                                                        \
>> >     if (mask & [some_modparam_var])                 \
>> >             pr_debug(fmt, ##__VA_ARGS__);           \
>> > } while (0)
>> > It might be useful to have standardized calls
>> > like pr_debug_level and pr_debug_mask instead
>> > of multiple hand-rolled variants treewide.
> []
>> looks interesting. I'm wondering how we handle module parameters though?
>> In the dynamic debug disabled case, we'd have to standardize the module
>> params names. And for the dynamic debug enabled case, I'm not sure how
>> we would honor those module params?
>
> One possibility would be to require a #define
> before use.
>
> Something like the DEBUG_VARIABLE used in
> drivers/media/ and include/media/.
>
> Maybe something like:
>
> #define DYNAMIC_DEBUG_LEVEL_VARIABLE module_var_foo
> and/or
> #define DYNAMIC_DEBUG_MASK_VARIABLE module_var_bar
>
> and
>
> $ echo 'module <foo> mask <n>' > <debugfs>/dynamic_debug/control
> and
> $ echo 'module <foo> level <n>' > <debugfs>/dynamic_debug/control
>
> could work to set the module control variable too.
>
>
>

how about just passing in condition directly ?

#define pr_dbg_if( cond, fmt, ... )                  \
do {                                                        \
    if (cond)                \
            pr_debug(fmt, ##__VA_ARGS__);           \
} while (0)

this will serve both mask and level, and could underlie both of them,
and its clear - no hidden checks against a variable named elsewhere.

Re: RFC: dynamic debug enhancements?

[Joe Perches]

On Mon, 2011-08-29 at 12:21 -0600, Jim Cromie wrote:
> how about just passing in condition directly ?
> #define pr_dbg_if( cond, fmt, ... )                  \
> do {                                                        \
>     if (cond)                \
>             pr_debug(fmt, ##__VA_ARGS__);           \
> } while (0)
> this will serve both mask and level, and could underlie both of them,
> and its clear - no hidden checks against a variable named elsewhere.

The idea is to let dynamic debug control the
output for the various mask/level tests.

That could get out of sync with pr_debug.

It separates the pr_debug condition from the test.