| Message ID | 20250210210109.3673582-1-alex.williamson@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI: Fix BUILD_BUG_ON usage for old gcc | expand |
On 02/10, Alex Williamson wrote: > > As reported in the below link, it seems older versions of gcc cannot > determine that the howmany variable is known for all callers. Include > a test so that newer compilers can still enforce this sanity check and > older compilers can still work. > > Fixes: 4453f360862e ("PCI: Batch BAR sizing operations") > Link: https://lore.kernel.org/all/20250209154512.GA18688@redhat.com > Reported-by: Oleg Nesterov <oleg@redhat.com> > Suggested-by: Oleg Nesterov <oleg@redhat.com> ^^^^^^^^^^^^ Well, thanks, but I didn't ;) > @@ -345,7 +345,8 @@ static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int rom) > unsigned int pos, reg; > u16 orig_cmd; > > - BUILD_BUG_ON(howmany > PCI_STD_NUM_BARS); > + BUILD_BUG_ON(__builtin_constant_p(howmany) && > + howmany > PCI_STD_NUM_BARS); Thanks! Tested-by: Oleg Nesterov <oleg@redhat.com>
On 02/10, Oleg Nesterov wrote: > > On 02/10, Alex Williamson wrote: > > > > @@ -345,7 +345,8 @@ static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int rom) > > unsigned int pos, reg; > > u16 orig_cmd; > > > > - BUILD_BUG_ON(howmany > PCI_STD_NUM_BARS); > > + BUILD_BUG_ON(__builtin_constant_p(howmany) && > > + howmany > PCI_STD_NUM_BARS); > > Thanks! > > Tested-by: Oleg Nesterov <oleg@redhat.com> Just in case... I agree with David, statically_true() looks a bit better and BUILD_BUG_ON(statically_true(howmany > PCI_STD_NUM_BARS)); also works for me, so if you decide to update this patch feel free to keep my Tested-by. Oleg.
On Ubuntu's (modern) GCC 13.3.0-6ubuntu2~24.04, both proposed options also build as expected for me when howmany is set correctly, and fail as expected when I force howmany = 7. Tested-by: Mitchell Augustin <mitchell.augustin@canonical.com> On Tue, Feb 11, 2025 at 4:05 AM Oleg Nesterov <oleg@redhat.com> wrote: > > On 02/10, Oleg Nesterov wrote: > > > > On 02/10, Alex Williamson wrote: > > > > > > @@ -345,7 +345,8 @@ static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int rom) > > > unsigned int pos, reg; > > > u16 orig_cmd; > > > > > > - BUILD_BUG_ON(howmany > PCI_STD_NUM_BARS); > > > + BUILD_BUG_ON(__builtin_constant_p(howmany) && > > > + howmany > PCI_STD_NUM_BARS); > > > > Thanks! > > > > Tested-by: Oleg Nesterov <oleg@redhat.com> > > Just in case... I agree with David, statically_true() looks a bit > better and > > BUILD_BUG_ON(statically_true(howmany > PCI_STD_NUM_BARS)); > > also works for me, so if you decide to update this patch feel free > to keep my Tested-by. > > Oleg. >
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index b6536ed599c3..1bde89d0dc0d 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -345,7 +345,8 @@ static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int rom) unsigned int pos, reg; u16 orig_cmd; - BUILD_BUG_ON(howmany > PCI_STD_NUM_BARS); + BUILD_BUG_ON(__builtin_constant_p(howmany) && + howmany > PCI_STD_NUM_BARS); if (dev->non_compliant_bars) return;
As reported in the below link, it seems older versions of gcc cannot determine that the howmany variable is known for all callers. Include a test so that newer compilers can still enforce this sanity check and older compilers can still work. Fixes: 4453f360862e ("PCI: Batch BAR sizing operations") Link: https://lore.kernel.org/all/20250209154512.GA18688@redhat.com Reported-by: Oleg Nesterov <oleg@redhat.com> Suggested-by: Oleg Nesterov <oleg@redhat.com> Signed-off-by: Alex Williamson <alex.williamson@redhat.com> --- Verified against gcc 14.2.1 to still trigger a build error if called with a constant value greater than 6, Oleg to confirm build issue is resolved for gcc 5.3.1. drivers/pci/probe.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)