Debug prints mac80211 drivers

Debug prints mac80211 drivers

[Krishna Chaitanya]

Hi,

What is the recommended method for adding
debug prints in mac80211 based drivers.

1) -DDEBUG + pr_debug ==> used by mac80211, brcm80211
2) -DDEBUG + dev_dbg ==> zd1201
3) dev_printk(KERN_DEBUG) ==> used by iwlwifi
4) printk(KERN_DEBUG) ==> Just to complete the list.

-- 
Thanks,
Regards,
Chaitanya T K.

Re: Debug prints mac80211 drivers

[Joe Perches]

On Fri, 2016-04-22 at 17:51 +0530, Krishna Chaitanya wrote:
> What is the recommended method for adding
> debug prints in mac80211 based drivers.
> 
> 1) -DDEBUG + pr_debug ==> used by mac80211, brcm80211
> 2) -DDEBUG + dev_dbg ==> zd1201
> 3) dev_printk(KERN_DEBUG) ==> used by iwlwifi
> 4) printk(KERN_DEBUG) ==> Just to complete the list.

wiphy_dbg -> netif_dbg -> netdev_dbg -> dev_dbg -> pr_debug

and CONFIG_DYNAMIC_DEBUG, no -DDEBUG required

Re: Debug prints mac80211 drivers

[Krishna Chaitanya]

On Sat, Apr 23, 2016 at 12:59 AM, Joe Perches <joe@perches.com> wrote:
>
> On Fri, 2016-04-22 at 17:51 +0530, Krishna Chaitanya wrote:
> > What is the recommended method for adding
> > debug prints in mac80211 based drivers.
> >
> > 1) -DDEBUG + pr_debug ==> used by mac80211, brcm80211
> > 2) -DDEBUG + dev_dbg ==> zd1201
> > 3) dev_printk(KERN_DEBUG) ==> used by iwlwifi
> > 4) printk(KERN_DEBUG) ==> Just to complete the list.
>
> wiphy_dbg -> netif_dbg -> netdev_dbg -> dev_dbg -> pr_debug
Ok, thats what checpatch --strict throws. but still different vendors follow
different standards, so wanted to check if we should go strictly with
checkpatch (or) is there any rationale behind choose each of the variant.

> and CONFIG_DYNAMIC_DEBUG, no -DDEBUG required
Yes, i understand. Till now we had this enabled, so pr_debug
works just fine, but now it is disabled hence the question.

Also there are pros and cons to having control using dyndbg,
user can disable dyndbg, there be missing imp debugs, in this case
having module level (-DDEBUG) helps but if we want entire system
to run in non-debug mode, disabling dyndbg helps.

Re: Debug prints mac80211 drivers

[Joe Perches]

On Sat, 2016-04-23 at 02:32 +0530, Krishna Chaitanya wrote:
> On Sat, Apr 23, 2016 at 12:59 AM, Joe Perches <joe@perches.com> wrote:
> > 
> > 
> > On Fri, 2016-04-22 at 17:51 +0530, Krishna Chaitanya wrote:
> > > 
> > > What is the recommended method for adding
> > > debug prints in mac80211 based drivers.
> > > 
> > > 1) -DDEBUG + pr_debug ==> used by mac80211, brcm80211
> > > 2) -DDEBUG + dev_dbg ==> zd1201
> > > 3) dev_printk(KERN_DEBUG) ==> used by iwlwifi
> > > 4) printk(KERN_DEBUG) ==> Just to complete the list.
> > wiphy_dbg -> netif_dbg -> netdev_dbg -> dev_dbg -> pr_debug
> Ok, thats what checpatch --strict throws. but still different vendors
> follow
> different standards, so wanted to check if we should go strictly with
> checkpatch (or) is there any rationale behind choose each of the
> variant.

Generally the variants are used to produce sufficient
logging information to identify the appropriate device.

Most all debugging printks shouldn't be emitted unless
actually debugging.

> > and CONFIG_DYNAMIC_DEBUG, no -DDEBUG required
> Yes, i understand. Till now we had this enabled, so pr_debug
> works just fine, but now it is disabled hence the question.
> 
> Also there are pros and cons to having control using dyndbg,
> user can disable dyndbg, there be missing imp

imp?

>  debugs, in this case
> having module level (-DDEBUG) helps but if we want entire system
> to run in non-debug mode, disabling dyndbg helps.

Confused:

dynamic debug printks aren't emitted by default
unless DEBUG is also defined or specifically
enabled by the user.

Re: Debug prints mac80211 drivers

[Krishna Chaitanya]

On Sat, Apr 23, 2016 at 3:48 AM, Joe Perches <joe@perches.com> wrote:
> On Sat, 2016-04-23 at 02:32 +0530, Krishna Chaitanya wrote:
>> On Sat, Apr 23, 2016 at 12:59 AM, Joe Perches <joe@perches.com> wrote:
>> >
>> >
>> > On Fri, 2016-04-22 at 17:51 +0530, Krishna Chaitanya wrote:
>> > >
>> > > What is the recommended method for adding
>> > > debug prints in mac80211 based drivers.
>> > >
>> > > 1) -DDEBUG + pr_debug ==> used by mac80211, brcm80211
>> > > 2) -DDEBUG + dev_dbg ==> zd1201
>> > > 3) dev_printk(KERN_DEBUG) ==> used by iwlwifi
>> > > 4) printk(KERN_DEBUG) ==> Just to complete the list.
>> > wiphy_dbg -> netif_dbg -> netdev_dbg -> dev_dbg -> pr_debug
>> Ok, thats what checpatch --strict throws. but still different vendors
>> follow
>> different standards, so wanted to check if we should go strictly with
>> checkpatch (or) is there any rationale behind choose each of the
>> variant.
>
> Generally the variants are used to produce sufficient
> logging information to identify the appropriate device.
>
> Most all debugging printks shouldn't be emitted unless
> actually debugging.
>
>> > and CONFIG_DYNAMIC_DEBUG, no -DDEBUG required
>> Yes, i understand. Till now we had this enabled, so pr_debug
>> works just fine, but now it is disabled hence the question.
>>
>> Also there are pros and cons to having control using dyndbg,
>> user can disable dyndbg, there be missing imp
>
> imp?
Sorry, important.
>>  debugs, in this case
>> having module level (-DDEBUG) helps but if we want entire system
>> to run in non-debug mode, disabling dyndbg helps.
>
> Confused:
>
> dynamic debug printks aren't emitted by default
> unless DEBUG is also defined or specifically
> enabled by the user.
I don't think so, enabling dynamic debug should suffice.

280 #if defined(CONFIG_DYNAMIC_DEBUG)
281 /* dynamic_pr_debug() uses pr_fmt() internally so we don't need it here */
282 #define pr_debug(fmt, ...) \
283         dynamic_pr_debug(fmt, ##__VA_ARGS__)
284 #elif defined(DEBUG)
285 #define pr_debug(fmt, ...) \
286         printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
287 #else
288 #define pr_debug(fmt, ...) \
289         no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
290 #endif



-- 
Thanks,
Regards,
Chaitanya T K.

Re: Debug prints mac80211 drivers

[Joe Perches]

On Sat, 2016-04-23 at 13:11 +0530, Krishna Chaitanya wrote:
> On Sat, Apr 23, 2016 at 3:48 AM, Joe Perches <joe@perches.com> wrote:
> > 
> > On Sat, 2016-04-23 at 02:32 +0530, Krishna Chaitanya wrote:
> > > 
> > > On Sat, Apr 23, 2016 at 12:59 AM, Joe Perches <joe@perches.com>
> > > wrote:
> > > > 
> > > > 
> > > > 
> > > > On Fri, 2016-04-22 at 17:51 +0530, Krishna Chaitanya wrote:
> > > > > 
> > > > > 
> > > > > What is the recommended method for adding
> > > > > debug prints in mac80211 based drivers.
> > > > > 
> > > > > 1) -DDEBUG + pr_debug ==> used by mac80211, brcm80211
> > > > > 2) -DDEBUG + dev_dbg ==> zd1201
> > > > > 3) dev_printk(KERN_DEBUG) ==> used by iwlwifi
> > > > > 4) printk(KERN_DEBUG) ==> Just to complete the list.
> > > > wiphy_dbg -> netif_dbg -> netdev_dbg -> dev_dbg -> pr_debug
> > > Ok, thats what checpatch --strict throws. but still different
> > > vendors
> > > follow
> > > different standards, so wanted to check if we should go strictly
> > > with
> > > checkpatch (or) is there any rationale behind choose each of the
> > > variant.
> > Generally the variants are used to produce sufficient
> > logging information to identify the appropriate device.
> > 
> > Most all debugging printks shouldn't be emitted unless
> > actually debugging.
> > 
> > > 
> > > > 
> > > > and CONFIG_DYNAMIC_DEBUG, no -DDEBUG required
> > > Yes, i understand. Till now we had this enabled, so pr_debug
> > > works just fine, but now it is disabled hence the question.
> > > 
> > > Also there are pros and cons to having control using dyndbg,
> > > user can disable dyndbg, there be missing imp
> > imp?
> Sorry, important.
> > 
> > > 
> > >  debugs, in this case
> > > having module level (-DDEBUG) helps but if we want entire system
> > > to run in non-debug mode, disabling dyndbg helps.
> > Confused:
> > 
> > dynamic debug printks aren't emitted by default
> > unless DEBUG is also defined or specifically
> > enabled by the user.
> I don't think so, enabling dynamic debug should suffice.
> 
> 280 #if defined(CONFIG_DYNAMIC_DEBUG)
> 281 /* dynamic_pr_debug() uses pr_fmt() internally so we don't need it
> here */
> 282 #define pr_debug(fmt, ...) \
> 283         dynamic_pr_debug(fmt, ##__VA_ARGS__)
> 284 #elif defined(DEBUG)
> 285 #define pr_debug(fmt, ...) \
> 286         printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> 287 #else
> 288 #define pr_debug(fmt, ...) \
> 289         no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> 290 #endif
> 

Nope.

Look at dynamic_debug.h

#if defined DEBUG
#define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINT
#else
#define _DPRINTK_FLAGS_DEFAULT 0
#endif

and

          .flags =  _DPRINTK_FLAGS_DEFAULT,

and

#define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt)		\
	static struct _ddebug  __aligned(8)			\
	__attribute__((section("__verbose"))) name = {		\
		.modname = KBUILD_MODNAME,			\
		.function = __func__,				\
		.filename = __FILE__,				\
		.format = (fmt),				\
		.lineno = __LINE__,				\
		.flags =  _DPRINTK_FLAGS_DEFAULT,		\
	}

and

#define dynamic_pr_debug(fmt, ...)				\
do {								\
	DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);		\
	if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT))	\
		__dynamic_pr_debug(&descriptor, pr_fmt(fmt),	\
				   ##__VA_ARGS__);		\
} while (0)

So by default, it's not enabled to be output

Re: Debug prints mac80211 drivers

[Krishna Chaitanya]

On Sat, Apr 23, 2016 at 1:35 PM, Joe Perches <joe@perches.com> wrote:
> On Sat, 2016-04-23 at 13:11 +0530, Krishna Chaitanya wrote:
>> On Sat, Apr 23, 2016 at 3:48 AM, Joe Perches <joe@perches.com> wrote:
>> >
>> > On Sat, 2016-04-23 at 02:32 +0530, Krishna Chaitanya wrote:
>> > >
>> > > On Sat, Apr 23, 2016 at 12:59 AM, Joe Perches <joe@perches.com>
>> > > wrote:
>> > > >
>> > > >
>> > > >
>> > > > On Fri, 2016-04-22 at 17:51 +0530, Krishna Chaitanya wrote:
>> > > > >
>> > > > >
>> > > > > What is the recommended method for adding
>> > > > > debug prints in mac80211 based drivers.
>> > > > >
>> > > > > 1) -DDEBUG + pr_debug ==> used by mac80211, brcm80211
>> > > > > 2) -DDEBUG + dev_dbg ==> zd1201
>> > > > > 3) dev_printk(KERN_DEBUG) ==> used by iwlwifi
>> > > > > 4) printk(KERN_DEBUG) ==> Just to complete the list.
>> > > > wiphy_dbg -> netif_dbg -> netdev_dbg -> dev_dbg -> pr_debug
>> > > Ok, thats what checpatch --strict throws. but still different
>> > > vendors
>> > > follow
>> > > different standards, so wanted to check if we should go strictly
>> > > with
>> > > checkpatch (or) is there any rationale behind choose each of the
>> > > variant.
>> > Generally the variants are used to produce sufficient
>> > logging information to identify the appropriate device.
>> >
>> > Most all debugging printks shouldn't be emitted unless
>> > actually debugging.
>> >
>> > >
>> > > >
>> > > > and CONFIG_DYNAMIC_DEBUG, no -DDEBUG required
>> > > Yes, i understand. Till now we had this enabled, so pr_debug
>> > > works just fine, but now it is disabled hence the question.
>> > >
>> > > Also there are pros and cons to having control using dyndbg,
>> > > user can disable dyndbg, there be missing imp
>> > imp?
>> Sorry, important.
>> >
>> > >
>> > >  debugs, in this case
>> > > having module level (-DDEBUG) helps but if we want entire system
>> > > to run in non-debug mode, disabling dyndbg helps.
>> > Confused:
>> >
>> > dynamic debug printks aren't emitted by default
>> > unless DEBUG is also defined or specifically
>> > enabled by the user.
>> I don't think so, enabling dynamic debug should suffice.
>>
>> 280 #if defined(CONFIG_DYNAMIC_DEBUG)
>> 281 /* dynamic_pr_debug() uses pr_fmt() internally so we don't need it
>> here */
>> 282 #define pr_debug(fmt, ...) \
>> 283         dynamic_pr_debug(fmt, ##__VA_ARGS__)
>> 284 #elif defined(DEBUG)
>> 285 #define pr_debug(fmt, ...) \
>> 286         printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
>> 287 #else
>> 288 #define pr_debug(fmt, ...) \
>> 289         no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
>> 290 #endif
>>
>
> Nope.
>
> Look at dynamic_debug.h
>
> #if defined DEBUG
> #define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINT
> #else
> #define _DPRINTK_FLAGS_DEFAULT 0
> #endif
>
> and
>
>           .flags =  _DPRINTK_FLAGS_DEFAULT,
>
> and
>
> #define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt)                \
>         static struct _ddebug  __aligned(8)                     \
>         __attribute__((section("__verbose"))) name = {          \
>                 .modname = KBUILD_MODNAME,                      \
>                 .function = __func__,                           \
>                 .filename = __FILE__,                           \
>                 .format = (fmt),                                \
>                 .lineno = __LINE__,                             \
>                 .flags =  _DPRINTK_FLAGS_DEFAULT,               \
>         }
>
> and
>
> #define dynamic_pr_debug(fmt, ...)                              \
> do {                                                            \
>         DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);         \
>         if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT))  \
>                 __dynamic_pr_debug(&descriptor, pr_fmt(fmt),    \
>                                    ##__VA_ARGS__);              \
> } while (0)
>
> So by default, it's not enabled to be output
Ok, i understand. We did not advertise -DDEBUG but pr_debug still
works, need to check if we are enabling it through some other option.

Re: Debug prints mac80211 drivers

[Krishna Chaitanya]

On Sat, Apr 23, 2016 at 1:53 PM, Krishna Chaitanya
<chaitanya.mgit@gmail.com> wrote:
> On Sat, Apr 23, 2016 at 1:35 PM, Joe Perches <joe@perches.com> wrote:
>> On Sat, 2016-04-23 at 13:11 +0530, Krishna Chaitanya wrote:
>>> On Sat, Apr 23, 2016 at 3:48 AM, Joe Perches <joe@perches.com> wrote:
>>> >
>>> > On Sat, 2016-04-23 at 02:32 +0530, Krishna Chaitanya wrote:
>>> > >
>>> > > On Sat, Apr 23, 2016 at 12:59 AM, Joe Perches <joe@perches.com>
>>> > > wrote:
>>> > > >
>>> > > >
>>> > > >
>>> > > > On Fri, 2016-04-22 at 17:51 +0530, Krishna Chaitanya wrote:
>>> > > > >
>>> > > > >
>>> > > > > What is the recommended method for adding
>>> > > > > debug prints in mac80211 based drivers.
>>> > > > >
>>> > > > > 1) -DDEBUG + pr_debug ==> used by mac80211, brcm80211
>>> > > > > 2) -DDEBUG + dev_dbg ==> zd1201
>>> > > > > 3) dev_printk(KERN_DEBUG) ==> used by iwlwifi
>>> > > > > 4) printk(KERN_DEBUG) ==> Just to complete the list.
>>> > > > wiphy_dbg -> netif_dbg -> netdev_dbg -> dev_dbg -> pr_debug
>>> > > Ok, thats what checpatch --strict throws. but still different
>>> > > vendors
>>> > > follow
>>> > > different standards, so wanted to check if we should go strictly
>>> > > with
>>> > > checkpatch (or) is there any rationale behind choose each of the
>>> > > variant.
>>> > Generally the variants are used to produce sufficient
>>> > logging information to identify the appropriate device.
>>> >
>>> > Most all debugging printks shouldn't be emitted unless
>>> > actually debugging.
>>> >
>>> > >
>>> > > >
>>> > > > and CONFIG_DYNAMIC_DEBUG, no -DDEBUG required
>>> > > Yes, i understand. Till now we had this enabled, so pr_debug
>>> > > works just fine, but now it is disabled hence the question.
>>> > >
>>> > > Also there are pros and cons to having control using dyndbg,
>>> > > user can disable dyndbg, there be missing imp
>>> > imp?
>>> Sorry, important.
>>> >
>>> > >
>>> > >  debugs, in this case
>>> > > having module level (-DDEBUG) helps but if we want entire system
>>> > > to run in non-debug mode, disabling dyndbg helps.
>>> > Confused:
>>> >
>>> > dynamic debug printks aren't emitted by default
>>> > unless DEBUG is also defined or specifically
>>> > enabled by the user.
>>> I don't think so, enabling dynamic debug should suffice.
>>>
>>> 280 #if defined(CONFIG_DYNAMIC_DEBUG)
>>> 281 /* dynamic_pr_debug() uses pr_fmt() internally so we don't need it
>>> here */
>>> 282 #define pr_debug(fmt, ...) \
>>> 283         dynamic_pr_debug(fmt, ##__VA_ARGS__)
>>> 284 #elif defined(DEBUG)
>>> 285 #define pr_debug(fmt, ...) \
>>> 286         printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
>>> 287 #else
>>> 288 #define pr_debug(fmt, ...) \
>>> 289         no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
>>> 290 #endif
>>>
>>
>> Nope.
>>
>> Look at dynamic_debug.h
>>
>> #if defined DEBUG
>> #define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINT
>> #else
>> #define _DPRINTK_FLAGS_DEFAULT 0
>> #endif
>>
>> and
>>
>>           .flags =  _DPRINTK_FLAGS_DEFAULT,
>>
>> and
>>
>> #define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt)                \
>>         static struct _ddebug  __aligned(8)                     \
>>         __attribute__((section("__verbose"))) name = {          \
>>                 .modname = KBUILD_MODNAME,                      \
>>                 .function = __func__,                           \
>>                 .filename = __FILE__,                           \
>>                 .format = (fmt),                                \
>>                 .lineno = __LINE__,                             \
>>                 .flags =  _DPRINTK_FLAGS_DEFAULT,               \
>>         }
>>
>> and
>>
>> #define dynamic_pr_debug(fmt, ...)                              \
>> do {                                                            \
>>         DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);         \
>>         if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT))  \
>>                 __dynamic_pr_debug(&descriptor, pr_fmt(fmt),    \
>>                                    ##__VA_ARGS__);              \
>> } while (0)
>>
>> So by default, it's not enabled to be output
> Ok, i understand. We did not advertise -DDEBUG but pr_debug still
> works, need to check if we are enabling it through some other option.
To conclude, for a device driver using option#2: -DDEBUG + dev_dbg should
suffice?