| Message ID | 1405498124-11519-1-git-send-email-qiudayu@linux.vnet.ibm.com |
|---|---|
| State | Not Applicable |
| Headers | show |
On Wed, Jul 16, 2014 at 2:08 AM, Mike Qiu <qiudayu@linux.vnet.ibm.com> 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> Hi Mike, How did you reproduce this build error? There are other include files that use pci_dev without a previous declaration or include of include/linux/pci.h, e.g., include/linux/msi.h, so I want to make sure we don't have similar problems elsewhere. 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; > -- > 1.8.1.4 > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/17/2014 02:08 AM, Bjorn Helgaas wrote: > On Wed, Jul 16, 2014 at 2:08 AM, Mike Qiu <qiudayu@linux.vnet.ibm.com> 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> > Hi Mike, > > How did you reproduce this build error? There are other include files > that use pci_dev without a previous declaration or include of > include/linux/pci.h, e.g., include/linux/msi.h, so I want to make sure > we don't have similar problems elsewhere. ( I have reply it before but seems haven't send out successful, just resend it, if you have seen, pls ignore this) I checkout to linux-next master branch, see the top commit is: commit b997a07604562f1a54cc531fe1cf7447f0ed6078 Author: Stephen Rothwell <sfr@canb.auug.org.au> Date: Tue Jul 15 18:34:28 2014 +1000 Add linux-next specific files for 20140715 Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> Use default config file(copy from /boot/) then use command "make -j60", and it will show that message. but for "msi.h" I haven't see this warning. Thanks, Mike > 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; >> -- >> 1.8.1.4 >> -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[+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 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; > -- > 1.8.1.4 > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; >> --
On 07/16/2014 01:08 AM, Mike Qiu wrote: > build log: > When CONFIG_PCIEAER is not enabled: > 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> Acked-by: Randy Dunlap <rdunlap@infradead.org> Tested-by: Randy Dunlap <rdunlap@infradead.org> > --- > 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; >
On Mon, Jul 21, 2014 at 10:31 PM, Randy Dunlap <rdunlap@infradead.org> wrote: > 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] Yeah, I should have said *an* x86 config doesn't see the problem. There are lots of config variable involved in that path. >> 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. Actually, since 0a2409aad38e is still in -next, the fix could be merged first to avoid the bisection trap here. But I don't know how Tony feels about rebasing his branches. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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;
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> --- include/linux/aer.h | 2 ++ 1 file changed, 2 insertions(+)