On Thu, Apr 26, 2012 at 6:02 AM, Luis R. Rodriguez <mcgrof@xxxxxxxxxxxxxxxx> wrote: > On Thu, Apr 26, 2012 at 5:23 AM, Luis R. Rodriguez > <mcgrof@xxxxxxxxxxxxxxxx> wrote: >> On Mon, Apr 23, 2012 at 9:47 PM, David Miller <davem@xxxxxxxxxxxxx> wrote: >>> From: "Luis R. Rodriguez" <mcgrof@xxxxxxxxxxxxxxxx> >>> Date: Mon, 23 Apr 2012 19:46:43 -0700 >>> >>>> From: "Luis R. Rodriguez" <mcgrof@xxxxxxxxxxxxxxxx> >>>> >>>> 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@xxxxxxxxxxxxxxxx> >>> >>> 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. OK I found no pr_fmt warnings at all for make allmodconfig ; make V=1 ; on v3.4-rc4. For this and others reason stated then yes, this patch is simply bogus. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
