Re: [PATCH] Fix build warnings in aer.h

[Randy Dunlap <rdunlap@infradead.org>]

On 07/21/2014 07:38 PM, Bjorn Helgaas wrote:
> [+cc Tuomas, Randy, Gavin, Gong, Lance, Tony]
> 
> On Wed, Jul 16, 2014 at 04:08:44AM -0400, Mike Qiu wrote:
>> build log:
>>
>> In file included from include/ras/ras_event.h:11:0,
>>                  from drivers/ras/ras.c:13:
>> include/linux/aer.h:42:129: warning: ‘struct pci_dev’
>> declared inside parameter list [enabled by default]
>>
>> include/linux/aer.h:42:129: warning: its scope is only
>> this definition or declaration, which is probably not
>> what you want [enabled by default]
>>
>> include/linux/aer.h:46:130: warning: ‘struct pci_dev’
>> declared inside parameter list [enabled by default]
>>
>> include/linux/aer.h:50:136: warning: ‘struct pci_dev’
>> declared inside parameter list [enabled by default]
>>
>> include/linux/aer.h:57:14: warning: ‘struct pci_dev’
>> declared inside parameter list [enabled by default]
>>
>> Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
> 
> OK, so the problem doesn't occur on x86 because there we have a struct
> pci_dev declaration reached earlier via:
> 
>   drivers/ras/ras.c
>     include/ras/ras_event.h
>       include/linux/edac.h
>         include/linux/device.h
>           include/linux/gfp.h
>             arch/x86/include/asm/mmzone.h
>               arch/x86/include/asm/mmzone_64.h
>                 arch/x86/include/asm/smp.h
>                   arch/x86/include/asm/mpspec.h
>                     arch/x86/include/asm/x86_init.h
>                       struct pci_dev;
>       include/linux/aer.h
>         ... pci_enable_pcie_error_reporting(struct pci_dev *dev);
> 
> You're building powerpc, which doesn't have the same pci_dev
> declaration in the arch header files, so you see the declaration
> problem in aer.h.

I get in on many i386 and x86_64 randconfig builds.... [checking]

20140715: 2 of 10 i386 randconfig builds
20140716: 1 of 10 i386 randconfig builds
20140717: 2 of 10 i386 randconfig builds
20140718: 1 of 10 i386 randconfig builds

20140715: 3 of 10 x86_64 randconfig builds
20140716: 1 of 10 x86_64 randconfig builds
20140717: 1 of 10 x86_64 randconfig builds
20140718: 2 of 10 x86_64 randconfig builds
20140721: 1 of 10 x86_64 randconfig builds


> I could apply your fix, but it should go in v3.16 along with the commit
> (0a2409aad38e ("trace, AER: Move trace into unified interface")) that
> exposes the problem, and I'm leaving on vacation before I can get this
> into -next, have it smoke-tested, and ask Linus to pull it.
> 
> So you might want to ask Tony to put in his tree, since it looks like
> he merged 0a2409aad38e in the first place.
> 
> Please add these:
> 
>   Fixes: 0a2409aad38e ("trace, AER: Move trace into unified interface")
>   Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> 
> I know 0a2409aad38e isn't actually *responsible* for the problem, but
> anybody who backports 0a2409aad38e should pick up this fix as well.
> 
> Lance, I added you because of an unrelated issue I noticed while reviewing
> this: you added the AER trace events with 1ca1d8d54f92 ("aerdrv: Trace
> Event for PCI Express Advanced Error Reporting"):
> 
>   +#define aer_correctable_errors         \
>   +       {BIT(0),        "Receiver Error"},              \
>   ...
> 
> I'd like to see all those "BIT(...)" things changed to use the #defines
> that already exist in include/uapi/linux/pci_regs.h, e.g.,
> PCI_ERR_COR_RCVR.  That way grep will find these uses, which will make
> maintenance easier.  There's no hurry about this part, of course.
> 
> Bjorn
> 
>> ---
>>  include/linux/aer.h | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/include/linux/aer.h b/include/linux/aer.h
>> index 4dbaa70..c826d1c 100644
>> --- a/include/linux/aer.h
>> +++ b/include/linux/aer.h
>> @@ -11,6 +11,8 @@
>>  #define AER_FATAL                        1
>>  #define AER_CORRECTABLE                        2
>>  
>> +struct pci_dev;
>> +
>>  struct aer_header_log_regs {
>>          unsigned int dw0;
>>          unsigned int dw1;
>> -- 


-- 
~Randy