* Re: Patch "PCI: Use pci_release_resource() instead of release_resource()" has been added to the 6.17-stable tree
[not found] <20251012141313.2893909-1-sashal@kernel.org>
@ 2025-10-13 9:20 ` Ilpo Järvinen
0 siblings, 0 replies; only message in thread
From: Ilpo Järvinen @ 2025-10-13 9:20 UTC (permalink / raw)
To: stable; +Cc: stable-commits, Bjorn Helgaas
[-- Attachment #1: Type: text/plain, Size: 7159 bytes --]
On Sun, 12 Oct 2025, Sasha Levin wrote:
> This is a note to let you know that I've just added the patch titled
>
> PCI: Use pci_release_resource() instead of release_resource()
>
> to the 6.17-stable tree which can be found at:
> http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
>
> The filename of the patch is:
> pci-use-pci_release_resource-instead-of-release_reso.patch
> and it can be found in the queue-6.17 subdirectory.
>
> If you, or anyone else, feels it should not be added to the stable tree,
> please let <stable@vger.kernel.org> know about it.
Hi Sasha and other stable maintainers,
I'm not sure why this is being selected for stable. AFAIK, it doesn't
"fix" something but is just a cleanup / consistency improvement.
It also makes a subtle change into flags behavior which has some
regression potential (although I estimate the likelihoods of problems is
low for this change).
--
i.
> commit a5c01cacc40950fb11cd299334cfbaf0542be443
> Author: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Date: Fri Aug 29 16:10:57 2025 +0300
>
> PCI: Use pci_release_resource() instead of release_resource()
>
> [ Upstream commit 3baeae36039afc233d4a42d6ff4aa7019892619f ]
>
> A few places in setup-bus.c call release_resource() directly and end up
> duplicating functionality from pci_release_resource() such as parent check,
> logging, and clearing the resource. Worse yet, the way the resource is
> cleared is inconsistent between different sites.
>
> Convert release_resource() calls into pci_release_resource() to remove code
> duplication. This will also make the resource start, end, and flags
> behavior consistent, i.e., start address is cleared, and only
> IORESOURCE_UNSET is asserted for the resource.
>
> While at it, eliminate the unnecessary initialization of idx variable in
> pci_bridge_release_resources().
>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Link: https://patch.msgid.link/20250829131113.36754-9-ilpo.jarvinen@linux.intel.com
> Stable-dep-of: 8278c6914306 ("PCI: Preserve bridge window resource type flags")
> Signed-off-by: Sasha Levin <sashal@kernel.org>
>
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index c992707a8ebd6..203c8ebef7029 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -431,8 +431,6 @@ static void __assign_resources_sorted(struct list_head *head,
> struct pci_dev_resource *dev_res, *tmp_res, *dev_res2;
> struct resource *res;
> struct pci_dev *dev;
> - const char *res_name;
> - int idx;
> unsigned long fail_type;
> resource_size_t add_align, align;
>
> @@ -540,14 +538,7 @@ static void __assign_resources_sorted(struct list_head *head,
> res = dev_res->res;
> dev = dev_res->dev;
>
> - if (!res->parent)
> - continue;
> -
> - idx = pci_resource_num(dev, res);
> - res_name = pci_resource_name(dev, idx);
> - pci_dbg(dev, "%s %pR: releasing\n", res_name, res);
> -
> - release_resource(res);
> + pci_release_resource(dev, pci_resource_num(dev, res));
> restore_dev_resource(dev_res);
> }
> /* Restore start/end/flags from saved list */
> @@ -1716,7 +1707,7 @@ static void pci_bridge_release_resources(struct pci_bus *bus,
> struct resource *r;
> unsigned int old_flags;
> struct resource *b_res;
> - int idx = 1;
> + int idx, ret;
>
> b_res = &dev->resource[PCI_BRIDGE_RESOURCES];
>
> @@ -1750,21 +1741,18 @@ static void pci_bridge_release_resources(struct pci_bus *bus,
>
> /* If there are children, release them all */
> release_child_resources(r);
> - if (!release_resource(r)) {
> - type = old_flags = r->flags & PCI_RES_TYPE_MASK;
> - pci_info(dev, "resource %d %pR released\n",
> - PCI_BRIDGE_RESOURCES + idx, r);
> - /* Keep the old size */
> - resource_set_range(r, 0, resource_size(r));
> - r->flags = 0;
>
> - /* Avoiding touch the one without PREF */
> - if (type & IORESOURCE_PREFETCH)
> - type = IORESOURCE_PREFETCH;
> - __pci_setup_bridge(bus, type);
> - /* For next child res under same bridge */
> - r->flags = old_flags;
> - }
> + type = old_flags = r->flags & PCI_RES_TYPE_MASK;
> + ret = pci_release_resource(dev, PCI_BRIDGE_RESOURCES + idx);
> + if (ret)
> + return;
> +
> + /* Avoiding touch the one without PREF */
> + if (type & IORESOURCE_PREFETCH)
> + type = IORESOURCE_PREFETCH;
> + __pci_setup_bridge(bus, type);
> + /* For next child res under same bridge */
> + r->flags = old_flags;
> }
>
> enum release_type {
> @@ -2409,7 +2397,6 @@ int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type)
> for (i = PCI_BRIDGE_RESOURCES; i < PCI_BRIDGE_RESOURCE_END;
> i++) {
> struct resource *res = &bridge->resource[i];
> - const char *res_name = pci_resource_name(bridge, i);
>
> if ((res->flags ^ type) & PCI_RES_TYPE_MASK)
> continue;
> @@ -2422,12 +2409,7 @@ int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type)
> if (ret)
> goto cleanup;
>
> - pci_info(bridge, "%s %pR: releasing\n", res_name, res);
> -
> - if (res->parent)
> - release_resource(res);
> - res->start = 0;
> - res->end = 0;
> + pci_release_resource(bridge, i);
> break;
> }
> if (i == PCI_BRIDGE_RESOURCE_END)
> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index d2b3ed51e8804..0468c058b5987 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
> @@ -406,20 +406,25 @@ int pci_reassign_resource(struct pci_dev *dev, int resno,
> return 0;
> }
>
> -void pci_release_resource(struct pci_dev *dev, int resno)
> +int pci_release_resource(struct pci_dev *dev, int resno)
> {
> struct resource *res = pci_resource_n(dev, resno);
> const char *res_name = pci_resource_name(dev, resno);
> + int ret;
>
> if (!res->parent)
> - return;
> + return 0;
>
> pci_info(dev, "%s %pR: releasing\n", res_name, res);
>
> - release_resource(res);
> + ret = release_resource(res);
> + if (ret)
> + return ret;
> res->end = resource_size(res) - 1;
> res->start = 0;
> res->flags |= IORESOURCE_UNSET;
> +
> + return 0;
> }
> EXPORT_SYMBOL(pci_release_resource);
>
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 59876de13860d..275df40587672 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1417,7 +1417,7 @@ void pci_reset_secondary_bus(struct pci_dev *dev);
> void pcibios_reset_secondary_bus(struct pci_dev *dev);
> void pci_update_resource(struct pci_dev *dev, int resno);
> int __must_check pci_assign_resource(struct pci_dev *dev, int i);
> -void pci_release_resource(struct pci_dev *dev, int resno);
> +int pci_release_resource(struct pci_dev *dev, int resno);
> static inline int pci_rebar_bytes_to_size(u64 bytes)
> {
> bytes = roundup_pow_of_two(bytes);
>
^ permalink raw reply [flat|nested] only message in thread