Return-path: Received: from smtprelay0013.hostedemail.com ([216.40.44.13]:58856 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751363AbcDWIFX (ORCPT ); Sat, 23 Apr 2016 04:05:23 -0400 Message-ID: <1461398719.2726.22.camel@perches.com> (sfid-20160423_100540_212319_77EC9DA2) Subject: Re: Debug prints mac80211 drivers From: Joe Perches To: Krishna Chaitanya Cc: linux-wireless , Johannes Berg Date: Sat, 23 Apr 2016 01:05:19 -0700 In-Reply-To: References: <1461353341.2726.14.camel@perches.com> <1461363503.2726.17.camel@perches.com> Content-Type: text/plain; charset="ISO-8859-1" Mime-Version: 1.0 Sender: linux-wireless-owner@vger.kernel.org List-ID: On Sat, 2016-04-23 at 13:11 +0530, Krishna Chaitanya wrote: > On Sat, Apr 23, 2016 at 3:48 AM, Joe Perches wrote: > > > > On Sat, 2016-04-23 at 02:32 +0530, Krishna Chaitanya wrote: > > > > > > On Sat, Apr 23, 2016 at 12:59 AM, Joe Perches > > > 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