From: Bjorn Helgaas <helgaas@kernel.org>
To: Jonas Gorski <jonas.gorski@gmail.com>
Cc: "Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
"Krzysztof Wilczyński" <kw@linux.com>,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
"Mickaël Salaün" <mic@digikod.net>,
"Rich Felker" <dalias@libc.org>,
linux-sh@vger.kernel.org,
"Dominik Brodowski" <linux@dominikbrodowski.net>,
"Andrew Lunn" <andrew@lunn.ch>,
sparclinux@vger.kernel.org,
"Stefano Stabellini" <sstabellini@kernel.org>,
"Yoshinori Sato" <ysato@users.sourceforge.jp>,
"Gregory Clement" <gregory.clement@bootlin.com>,
"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
"Russell King" <linux@armlinux.org.uk>,
linux-acpi@vger.kernel.org, "Miguel Ojeda" <ojeda@kernel.org>,
xen-devel@lists.xenproject.org,
"Matt Turner" <mattst88@gmail.com>,
"Anatolij Gustschin" <agust@denx.de>,
"Sebastian Hesselbarth" <sebastian.hesselbarth@gmail.com>,
"Arnd Bergmann" <arnd@arndb.de>,
"Niklas Schnelle" <schnelle@linux.ibm.com>,
"Richard Henderson" <richard.henderson@linaro.org>,
"Nicholas Piggin" <npiggin@gmail.com>,
"Ivan Kokshaysky" <ink@jurassic.park.msu.ru>,
"John Paul Adrian Glaubitz" <glaubitz@physik.fu-berlin.de>,
"Bjorn Helgaas" <bhelgaas@google.com>,
"Mika Westerberg" <mika.westerberg@linux.intel.com>,
linux-arm-kernel@lists.infradead.org,
"Juergen Gross" <jgross@suse.com>,
"Thomas Bogendoerfer" <tsbogend@alpha.franken.de>,
"Philippe Mathieu-Daudé" <philmd@linaro.org>,
linuxppc-dev@lists.ozlabs.org,
"Randy Dunlap" <rdunlap@infradead.org>,
linux-mips@vger.kernel.org,
"Oleksandr Tyshchenko" <oleksandr_tyshchenko@epam.com>,
linux-alpha@vger.kernel.org, "Pali Rohár" <pali@kernel.org>,
"David S. Miller" <davem@davemloft.net>,
"Maciej W. Rozycki" <macro@orcam.me.uk>
Subject: Re: [PATCH v8 0/7] Add pci_dev_for_each_resource() helper and update users
Date: Wed, 31 May 2023 16:30:28 -0500 [thread overview]
Message-ID: <ZHe8dKb3f392MfBO@bhelgaas> (raw)
In-Reply-To: <CAOiHx==5YWhDiZP2PyHZiJrmtqRzvqCqoSO59RwuYuR85BezBg@mail.gmail.com>
On Wed, May 31, 2023 at 08:48:35PM +0200, Jonas Gorski wrote:
> ...
> Looking at the code I understand where coverity is coming from:
>
> #define __pci_dev_for_each_res0(dev, res, ...) \
> for (unsigned int __b = 0; \
> res = pci_resource_n(dev, __b), __b < PCI_NUM_RESOURCES; \
> __b++)
>
> res will be assigned before __b is checked for being less than
> PCI_NUM_RESOURCES, making it point to behind the array at the end of
> the last loop iteration.
>
> Rewriting the test expression as
>
> __b < PCI_NUM_RESOURCES && (res = pci_resource_n(dev, __b));
>
> should avoid the (coverity) warning by making use of lazy evaluation.
>
> It probably makes the code slightly less performant as res will now be
> checked for being not NULL (which will always be true), but I doubt it
> will be significant (or in any hot paths).
Thanks a lot for looking into this! I think you're right, and I think
the rewritten expression is more logical as well. Do you want to post
a patch for it?
Bjorn
WARNING: multiple messages have this Message-ID (diff)
From: Bjorn Helgaas <helgaas@kernel.org>
To: Jonas Gorski <jonas.gorski@gmail.com>
Cc: "Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
"Krzysztof Wilczyński" <kw@linux.com>,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
"Mickaël Salaün" <mic@digikod.net>,
"Rich Felker" <dalias@libc.org>,
linux-sh@vger.kernel.org,
"Dominik Brodowski" <linux@dominikbrodowski.net>,
"Andrew Lunn" <andrew@lunn.ch>,
sparclinux@vger.kernel.org,
"Stefano Stabellini" <sstabellini@kernel.org>,
"Yoshinori Sato" <ysato@users.sourceforge.jp>,
"Gregory Clement" <gregory.clement@bootlin.com>,
"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
"Russell King" <linux@armlinux.org.uk>,
linux-acpi@vger.kernel.org, "Miguel Ojeda" <ojeda@kernel.org>,
xen-devel@lists.xenproject.org,
"Matt Turner" <mattst88@gmail.com>,
"Anatolij Gustschin" <agust@denx.de>,
"Sebastian Hesselbarth" <sebastian.hesselbarth@gmail.com>,
"Arnd Bergmann" <arnd@arndb.de>,
"Niklas Schnelle" <schnelle@linux.ibm.com>,
"Richard Henderson" <richard.henderson@linaro.org>,
"Nicholas Piggin" <npiggin@gmail.com>,
"Ivan Kokshaysky" <ink@jurassic.park.msu.ru>,
"John Paul Adrian Glaubitz" <glaubitz@physik.fu-berlin.de>,
"Bjorn Helgaas" <bhelgaas@google.com>,
"Mika Westerberg" <mika.westerberg@linux.intel.com>,
linux-arm-kernel@lists.infradead.org,
"Juergen Gross" <jgross@suse.com>,
"Thomas Bogendoerfer" <tsbogend@alpha.franken.de>,
"Philippe Mathieu-Daudé" <philmd@linaro.org>,
linuxppc-dev@lists.ozlabs.org,
"Randy Dunlap" <rdunlap@infradead.org>,
linux-mips@vger.kernel.org,
"Oleksandr Tyshchenko" <oleksandr_tyshchenko@epam.com>,
linux-alpha@vger.kernel.org, "Pali Rohár" <pali@kernel.org>,
"David S. Miller" <davem@davemloft.net>,
"Maciej W. Rozycki" <macro@orcam.me.uk>
Subject: Re: [PATCH v8 0/7] Add pci_dev_for_each_resource() helper and update users
Date: Wed, 31 May 2023 16:30:28 -0500 [thread overview]
Message-ID: <ZHe8dKb3f392MfBO@bhelgaas> (raw)
In-Reply-To: <CAOiHx==5YWhDiZP2PyHZiJrmtqRzvqCqoSO59RwuYuR85BezBg@mail.gmail.com>
On Wed, May 31, 2023 at 08:48:35PM +0200, Jonas Gorski wrote:
> ...
> Looking at the code I understand where coverity is coming from:
>
> #define __pci_dev_for_each_res0(dev, res, ...) \
> for (unsigned int __b = 0; \
> res = pci_resource_n(dev, __b), __b < PCI_NUM_RESOURCES; \
> __b++)
>
> res will be assigned before __b is checked for being less than
> PCI_NUM_RESOURCES, making it point to behind the array at the end of
> the last loop iteration.
>
> Rewriting the test expression as
>
> __b < PCI_NUM_RESOURCES && (res = pci_resource_n(dev, __b));
>
> should avoid the (coverity) warning by making use of lazy evaluation.
>
> It probably makes the code slightly less performant as res will now be
> checked for being not NULL (which will always be true), but I doubt it
> will be significant (or in any hot paths).
Thanks a lot for looking into this! I think you're right, and I think
the rewritten expression is more logical as well. Do you want to post
a patch for it?
Bjorn
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Bjorn Helgaas <helgaas@kernel.org>
To: Jonas Gorski <jonas.gorski@gmail.com>
Cc: "Krzysztof Wilczyński" <kw@linux.com>,
"Rich Felker" <dalias@libc.org>,
linux-sh@vger.kernel.org, linux-pci@vger.kernel.org,
"Dominik Brodowski" <linux@dominikbrodowski.net>,
linux-mips@vger.kernel.org, "Bjorn Helgaas" <bhelgaas@google.com>,
"Andrew Lunn" <andrew@lunn.ch>,
sparclinux@vger.kernel.org,
"Stefano Stabellini" <sstabellini@kernel.org>,
"Yoshinori Sato" <ysato@users.sourceforge.jp>,
"Gregory Clement" <gregory.clement@bootlin.com>,
"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
"Russell King" <linux@armlinux.org.uk>,
linux-acpi@vger.kernel.org, "Miguel Ojeda" <ojeda@kernel.org>,
xen-devel@lists.xenproject.org,
"Matt Turner" <mattst88@gmail.com>,
"Anatolij Gustschin" <agust@denx.de>,
"Sebastian Hesselbarth" <sebastian.hesselbarth@gmail.com>,
"Arnd Bergmann" <arnd@arndb.de>,
"Niklas Schnelle" <schnelle@linux.ibm.com>,
"Richard Henderson" <richard.henderson@linaro.org>,
"Nicholas Piggin" <npiggin@gmail.com>,
"Ivan Kokshaysky" <ink@jurassic.park.msu.ru>,
"John Paul Adrian Glaubitz" <glaubitz@physik.fu-berlin.de>,
"Mickaël Salaün" <mic@digikod.net>,
"Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
"Mika Westerberg" <mika.westerberg@linux.intel.com>,
linux-arm-kernel@lists.infradead.org,
"Juergen Gross" <jgross@suse.com>,
"Thomas Bogendoerfer" <tsbogend@alpha.franken.de>,
"Philippe Mathieu-Daudé" <philmd@linaro.org>,
"Pali Rohár" <pali@kernel.org>,
"Randy Dunlap" <rdunlap@infradead.org>,
linux-kernel@vger.kernel.org,
"Oleksandr Tyshchenko" <oleksandr_tyshchenko@epam.com>,
linux-alpha@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
"David S. Miller" <davem@davemloft.net>,
"Maciej W. Rozycki" <macro@orcam.me.uk>
Subject: Re: [PATCH v8 0/7] Add pci_dev_for_each_resource() helper and update users
Date: Wed, 31 May 2023 16:30:28 -0500 [thread overview]
Message-ID: <ZHe8dKb3f392MfBO@bhelgaas> (raw)
In-Reply-To: <CAOiHx==5YWhDiZP2PyHZiJrmtqRzvqCqoSO59RwuYuR85BezBg@mail.gmail.com>
On Wed, May 31, 2023 at 08:48:35PM +0200, Jonas Gorski wrote:
> ...
> Looking at the code I understand where coverity is coming from:
>
> #define __pci_dev_for_each_res0(dev, res, ...) \
> for (unsigned int __b = 0; \
> res = pci_resource_n(dev, __b), __b < PCI_NUM_RESOURCES; \
> __b++)
>
> res will be assigned before __b is checked for being less than
> PCI_NUM_RESOURCES, making it point to behind the array at the end of
> the last loop iteration.
>
> Rewriting the test expression as
>
> __b < PCI_NUM_RESOURCES && (res = pci_resource_n(dev, __b));
>
> should avoid the (coverity) warning by making use of lazy evaluation.
>
> It probably makes the code slightly less performant as res will now be
> checked for being not NULL (which will always be true), but I doubt it
> will be significant (or in any hot paths).
Thanks a lot for looking into this! I think you're right, and I think
the rewritten expression is more logical as well. Do you want to post
a patch for it?
Bjorn
WARNING: multiple messages have this Message-ID (diff)
From: Bjorn Helgaas <helgaas@kernel.org>
To: Jonas Gorski <jonas.gorski@gmail.com>
Cc: "Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
"Krzysztof Wilczyński" <kw@linux.com>,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
"Mickaël Salaün" <mic@digikod.net>,
"Rich Felker" <dalias@libc.org>,
linux-sh@vger.kernel.org,
"Dominik Brodowski" <linux@dominikbrodowski.net>,
"Andrew Lunn" <andrew@lunn.ch>,
sparclinux@vger.kernel.org,
"Stefano Stabellini" <sstabellini@kernel.org>,
"Yoshinori Sato" <ysato@users.sourceforge.jp>,
"Gregory Clement" <gregory.clement@bootlin.com>,
"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
"Russell King" <linux@armlinux.org.uk>,
linux-acpi@vger.kernel.org, "Miguel Ojeda" <ojeda@kernel.org>,
xen-devel@lists.xenproject.org,
"Matt Turner" <mattst88@gmail.com>,
"Anatolij Gustschin" <agust@denx.de>
Subject: Re: [PATCH v8 0/7] Add pci_dev_for_each_resource() helper and update users
Date: Wed, 31 May 2023 16:30:28 -0500 [thread overview]
Message-ID: <ZHe8dKb3f392MfBO@bhelgaas> (raw)
In-Reply-To: <CAOiHx==5YWhDiZP2PyHZiJrmtqRzvqCqoSO59RwuYuR85BezBg@mail.gmail.com>
On Wed, May 31, 2023 at 08:48:35PM +0200, Jonas Gorski wrote:
> ...
> Looking at the code I understand where coverity is coming from:
>
> #define __pci_dev_for_each_res0(dev, res, ...) \
> for (unsigned int __b = 0; \
> res = pci_resource_n(dev, __b), __b < PCI_NUM_RESOURCES; \
> __b++)
>
> res will be assigned before __b is checked for being less than
> PCI_NUM_RESOURCES, making it point to behind the array at the end of
> the last loop iteration.
>
> Rewriting the test expression as
>
> __b < PCI_NUM_RESOURCES && (res = pci_resource_n(dev, __b));
>
> should avoid the (coverity) warning by making use of lazy evaluation.
>
> It probably makes the code slightly less performant as res will now be
> checked for being not NULL (which will always be true), but I doubt it
> will be significant (or in any hot paths).
Thanks a lot for looking into this! I think you're right, and I think
the rewritten expression is more logical as well. Do you want to post
a patch for it?
Bjorn
next prev parent reply other threads:[~2023-05-31 21:30 UTC|newest]
Thread overview: 95+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-30 16:24 [PATCH v8 0/7] Add pci_dev_for_each_resource() helper and update users Andy Shevchenko
2023-03-30 16:24 ` Andy Shevchenko
2023-03-30 16:24 ` Andy Shevchenko
2023-03-30 16:24 ` Andy Shevchenko
2023-03-30 16:24 ` [PATCH v8 1/7] kernel.h: Split out COUNT_ARGS() and CONCATENATE() Andy Shevchenko
2023-03-30 16:24 ` Andy Shevchenko
2023-03-30 16:24 ` Andy Shevchenko
2023-03-30 16:24 ` Andy Shevchenko
2023-03-30 16:24 ` [PATCH v8 2/7] PCI: Introduce pci_resource_n() Andy Shevchenko
2023-03-30 16:24 ` Andy Shevchenko
2023-03-30 16:24 ` Andy Shevchenko
2023-03-30 16:24 ` Andy Shevchenko
2023-03-30 16:24 ` [PATCH v8 3/7] PCI: Introduce pci_dev_for_each_resource() Andy Shevchenko
2023-03-30 16:24 ` Andy Shevchenko
2023-03-30 16:24 ` Andy Shevchenko
2023-03-30 16:24 ` Andy Shevchenko
2023-03-30 16:24 ` [PATCH v8 4/7] PCI: Document pci_bus_for_each_resource() to avoid confusion Andy Shevchenko
2023-03-30 16:24 ` Andy Shevchenko
2023-03-30 16:24 ` Andy Shevchenko
2023-03-30 16:24 ` Andy Shevchenko
2023-03-30 16:24 ` [PATCH v8 5/7] PCI: Allow pci_bus_for_each_resource() to take less arguments Andy Shevchenko
2023-03-30 16:24 ` Andy Shevchenko
2023-03-30 16:24 ` Andy Shevchenko
2023-03-30 16:24 ` Andy Shevchenko
2023-04-05 11:50 ` Andy Shevchenko
2023-04-05 11:50 ` Andy Shevchenko
2023-04-05 11:50 ` Andy Shevchenko
2023-04-05 11:50 ` Andy Shevchenko
2023-04-05 20:11 ` Bjorn Helgaas
2023-04-05 20:11 ` Bjorn Helgaas
2023-04-05 20:11 ` Bjorn Helgaas
2023-04-05 20:11 ` Bjorn Helgaas
2023-03-30 16:24 ` [PATCH v8 6/7] EISA: Convert to use less arguments in pci_bus_for_each_resource() Andy Shevchenko
2023-03-30 16:24 ` Andy Shevchenko
2023-03-30 16:24 ` Andy Shevchenko
2023-03-30 16:24 ` Andy Shevchenko
2023-03-30 16:24 ` [PATCH v8 7/7] pcmcia: " Andy Shevchenko
2023-03-30 16:24 ` Andy Shevchenko
2023-03-30 16:24 ` Andy Shevchenko
2023-03-30 16:24 ` Andy Shevchenko
2023-04-05 8:30 ` Andy Shevchenko
2023-04-05 8:30 ` Andy Shevchenko
2023-04-05 8:30 ` Andy Shevchenko
2023-04-05 8:30 ` Andy Shevchenko
2023-04-04 16:11 ` [PATCH v8 0/7] Add pci_dev_for_each_resource() helper and update users Bjorn Helgaas
2023-04-04 16:11 ` Bjorn Helgaas
2023-04-04 16:11 ` Bjorn Helgaas
2023-04-04 16:11 ` Bjorn Helgaas
2023-04-05 8:28 ` Andy Shevchenko
2023-04-05 8:28 ` Andy Shevchenko
2023-04-05 8:28 ` Andy Shevchenko
2023-04-05 8:28 ` Andy Shevchenko
2023-04-05 20:18 ` Bjorn Helgaas
2023-04-05 20:18 ` Bjorn Helgaas
2023-04-05 20:18 ` Bjorn Helgaas
2023-04-05 20:18 ` Bjorn Helgaas
2023-04-06 10:31 ` Andy Shevchenko
2023-04-06 10:31 ` Andy Shevchenko
2023-04-06 10:31 ` Andy Shevchenko
2023-04-06 10:31 ` Andy Shevchenko
2023-05-09 18:21 ` Bjorn Helgaas
2023-05-09 18:21 ` Bjorn Helgaas
2023-05-09 18:21 ` Bjorn Helgaas
2023-05-12 10:56 ` Andy Shevchenko
2023-05-12 10:56 ` Andy Shevchenko
2023-05-12 10:56 ` Andy Shevchenko
2023-05-12 19:48 ` Bjorn Helgaas
2023-05-12 19:48 ` Bjorn Helgaas
2023-05-12 19:48 ` Bjorn Helgaas
2023-05-30 21:24 ` Bjorn Helgaas
2023-05-30 21:24 ` Bjorn Helgaas
2023-05-30 21:24 ` Bjorn Helgaas
2023-05-30 21:24 ` Bjorn Helgaas
2023-05-31 18:48 ` Jonas Gorski
2023-05-31 18:48 ` Jonas Gorski
2023-05-31 21:30 ` Bjorn Helgaas [this message]
2023-05-31 21:30 ` Bjorn Helgaas
2023-05-31 21:30 ` Bjorn Helgaas
2023-05-31 21:30 ` Bjorn Helgaas
2023-06-01 11:17 ` Jonas Gorski
2023-06-01 11:17 ` Jonas Gorski
2023-06-01 11:17 ` Jonas Gorski
2023-06-01 11:17 ` Jonas Gorski
2023-06-05 14:04 ` Andy Shevchenko
2023-06-05 14:04 ` Andy Shevchenko
2023-06-05 14:04 ` Andy Shevchenko
2023-06-05 14:04 ` Andy Shevchenko
2023-06-01 16:25 ` Andy Shevchenko
2023-06-01 16:25 ` Andy Shevchenko
2023-06-01 16:25 ` Andy Shevchenko
2023-06-01 16:25 ` Andy Shevchenko
2023-06-01 16:27 ` Andy Shevchenko
2023-06-01 16:27 ` Andy Shevchenko
2023-06-01 16:27 ` Andy Shevchenko
2023-06-01 16:27 ` Andy Shevchenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZHe8dKb3f392MfBO@bhelgaas \
--to=helgaas@kernel.org \
--cc=agust@denx.de \
--cc=andrew@lunn.ch \
--cc=andriy.shevchenko@linux.intel.com \
--cc=arnd@arndb.de \
--cc=bhelgaas@google.com \
--cc=dalias@libc.org \
--cc=davem@davemloft.net \
--cc=glaubitz@physik.fu-berlin.de \
--cc=gregory.clement@bootlin.com \
--cc=ink@jurassic.park.msu.ru \
--cc=jgross@suse.com \
--cc=jonas.gorski@gmail.com \
--cc=kw@linux.com \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-alpha@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mips@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linux-sh@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=linux@dominikbrodowski.net \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=macro@orcam.me.uk \
--cc=mattst88@gmail.com \
--cc=mic@digikod.net \
--cc=mika.westerberg@linux.intel.com \
--cc=npiggin@gmail.com \
--cc=ojeda@kernel.org \
--cc=oleksandr_tyshchenko@epam.com \
--cc=pali@kernel.org \
--cc=philmd@linaro.org \
--cc=rafael.j.wysocki@intel.com \
--cc=rdunlap@infradead.org \
--cc=richard.henderson@linaro.org \
--cc=schnelle@linux.ibm.com \
--cc=sebastian.hesselbarth@gmail.com \
--cc=sparclinux@vger.kernel.org \
--cc=sstabellini@kernel.org \
--cc=tsbogend@alpha.franken.de \
--cc=xen-devel@lists.xenproject.org \
--cc=ysato@users.sourceforge.jp \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.