Message-ID: <CAB=NE6ViDGHPn=OsqFK1rJb-5J+1xzvf3h9+Rd-MgmWpn-T2fA@mail.gmail.com>
Date:	Thu, 26 Apr 2012 06:02:17 -0700
From:	"Luis R. Rodriguez" <mcgrof@...not-panic.com>
To:	David Miller <davem@...emloft.net>
Cc:	zambrano@...adcom.com, netdev@...r.kernel.org,
	linux-wireless <linux-wireless@...r.kernel.org>,
	linux-bluetooth <linux-bluetooth@...r.kernel.org>,
	Arend Van Spriel <arend@...adcom.com>,
	Joe Perches <joe@...ches.com>
Subject: Re: [PATCH] b44: properly use pr_fmt()

On Thu, Apr 26, 2012 at 5:23 AM, Luis R. Rodriguez
<mcgrof@...not-panic.com> wrote:
> On Mon, Apr 23, 2012 at 9:47 PM, David Miller <davem@...emloft.net> wrote:
>> From: "Luis R. Rodriguez" <mcgrof@...not-panic.com>
>> Date: Mon, 23 Apr 2012 19:46:43 -0700
>>
>>> From: "Luis R. Rodriguez" <mcgrof@...not-panic.com>
>>>
>>> pr_fmt() is either defined or we redefine it. Typically
>>> drivers define it prior to including printk.h but this
>>> is done under the assumption that no other subsystem
>>> it uses has already defined pr_fmt(). In such cases
>>> pr_fmt() should be undefined and redefined.
>>>
>>> Doing this properly shaves down compilation time quite
>>> considerably.
>>>
>>> Signed-off-by: Luis R. Rodriguez <mcgrof@...not-panic.com>
>>
>> Every driver defines pr_fmt then includes the headers.  I do not
>> see any other place which performs the initial undef of pr_fmt
>> like you want to start doing here.
>
> One example is, and that is where I got the hint:
>
> drivers/net/wireless/mwifiex/decl.h
>
>> If there is a reason we should start doing this, it would appear
>> to be a universal reason, not one specific to this driver.
>
> Sure.
>
>> If that's the case, we should formally agree to this and then work
>> on gradually converting all the drivers in reasonably sized chunks.
>
> Understood.
>
>> Either way I can't apply this patch as-is, sorry Luis.
>
> So the issue can occur if any parent header ends up defining this,
> this could happen if a subsystem wants to define the pr_fmt in their
> own header and a driver for example wants to override this. In theory
> this does not occur but in practice this may vary and if we want to
> ensure this does not happen perhaps the thing to do is to document
> this as such.
>
> I just tested allmodconfig though and checked compilation for V=1
> against drivers/net/wireless/ and found no issues against linux-next
> though so in practice at least for wireless networking it seems we are
> squeaky clean in its usage. I'll test across the kernel though next to
> see if there are any real violators of the assumption. Unfortunately
> I've hit an issue with compiling allmodconfig already with linux-next
> that is already reported so my homework will be to find a shiny
> linux-next to compile test against allmodconfig.
>
> An area that this does clearly happen but that likely does not merit
> and upstream change for is in the backport work compat that defines
> this for older kernels and later we have to also backport users of
> pr_fmt such as pr_emerg_once. Ultimately drivers will want to actually
> then override this and in that case the undef is necessary as
> otherwise you get the redefinition warning which incurs in practice a
> considerable CPU performance hit at compilation time.

Additionally Arend has pointed pointed out that previously printk.h
was removed as a direct include as well given that kernel.h includes
it already:

https://lkml.org/lkml/2011/5/25/331

So this would be a second issue with this patch.

  Luis
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists