Re: [RFC PATCH] PCI: Sort resources by size as secondary key

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incomplete fix: `__assign_resources_sorted()` fails to maintain the new size-based secondary sort key when bridge windows are dynamically resized, breaking the sorting invariant.
--

commit b576a58abc2576977d567e69cb6a8b9e9d867d9b
Author: Ding Hui <dinghui@sangfor.com.cn>
Subject: PCI: Sort resources by size as secondary key

The commit updates pdev_sort_resources() to use resource size as a secondary
sorting key to reduce fragmentation. This resolves an issue on the BCM57414
NIC where function 1 failed to enable SR-IOV after remove and rescan due
to BAR allocation failures.

> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 4cf120ebe5adf..63f224f0c6bee 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -367,7 +367,8 @@ static void pdev_sort_resources(struct pci_dev *dev, struct list_head *head)
>  			align = pci_resource_alignment(dev_res->dev,
>  							 dev_res->res);
>  
> -			if (r_align > align) {
> +			if (r_align > align ||
> +			    (r_align == align && resource_size(r) > resource_size(dev_res->res))) {

[Severity: High]
Does this patch also need to update the re-sorting logic in
__assign_resources_sorted() to maintain the size-based secondary sort key?

When a bridge window requests additional size via realloc_head in
__assign_resources_sorted(), its size is increased:

drivers/pci/setup-bus.c:__assign_resources_sorted() {
    ...
    res->end += addsize_res->add_size;
    ...
    if (addsize_res->min_align <= res->start)
        continue;

If the alignment does not increase, it skips re-sorting entirely, leaving the
enlarged resource in its original position based on its older, smaller size.

If the alignment does increase, it is re-inserted into the sorted list, but
the loop only compares alignments:

    list_for_each_entry(dev_res2, head, list) {
        align = pci_resource_alignment(dev_res2->dev, dev_res2->res);
        if (addsize_res->min_align > align) {
            list_move_tail(&dev_res->list, &dev_res2->list);
            break;
        }
    }

This skips over elements with equal alignment, placing the newly enlarged
resource at the end of its alignment group and disregarding its size. Because
bridge windows are frequently dynamically resized, failing to sort them by
size when enlarged might place them behind smaller resources of the same
alignment, potentially re-introducing the fragmentation and allocation
failures this patch intends to solve.

>  				n = &dev_res->list;
>  				break;
>  			}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618072536.28199-1-dinghui@sangfor.com.cn?part=1