Re: [PATCH 2/4] PCI: add functionality for resizing resources v3

[Andy Shevchenko]

On Thu, May 4, 2017 at 12:23 PM, Christian KÃnig
<deathsimple@xxxxxxxxxxx> wrote:
> Am 26.04.2017 um 19:00 schrieb Andy Shevchenko:

>>> +       while (1) {
>>
>> This raises red flag. Care to refactor?
>> Also do {} while () syntax allows faster to get that the loop body
>> goes at least once.
>
>
> I've tried to refactor this, but couldn't come up with something which works
> and is readable at the same time.
>
> Any suggestion how this should look like?

This is original code.

--- 8< --- 8< ---

    while (1) {
            for (i = PCI_BRIDGE_RESOURCES; i < PCI_BRIDGE_RESOURCE_END; i++) {
                    struct resource *res = &bridge->resource[i];

                    if ((res->flags & type_mask) != (type & type_mask))
                            continue;

                    /* Ignore BARs which are still in use */
                    if (res->child)
                            continue;

                    ret = add_to_list(&saved, bridge, res, 0, 0);
                    if (ret)
                            goto cleanup;

                    if (res->parent)
                            release_resource(res);
                    res->start = 0;
                    res->end = 0;
                    dev_info(&bridge->dev, "BAR %d: released %pR\n", i, res);
                    break;
            }
            if (i == PCI_BRIDGE_RESOURCE_END)
                    break;

            if (!bridge->bus || !bridge->bus->self)
                    break;

            bridge = bridge->bus->self;
    }

--- 8< --- 8< ---

I would think about something like below

static int ...xxx...(...)
{
    unsigned int i;
    int ret;

    for (i = PCI_BRIDGE_RESOURCES; i < PCI_BRIDGE_RESOURCE_END; i++) {
            struct resource *res = &bridge->resource[i];

            /* Ignore BARs which are still in use */
            if (((res->flags ^ type) & type_mask) == 0 && !res->child)
                    break;
    }
    if (i == PCI_BRIDGE_RESOURCE_END)
        return -EBUSY;

    ret = add_to_list(&saved, bridge, res, 0, 0);
    if (ret)
            return ret;

    if (res->parent)
            release_resource(res);

    res->start = 0;
    res->end = 0;
    dev_info(&bridge->dev, "BAR %d: released %pR\n", i, res);
    return i;
}


struct pci_dev *next = bridge;
...
    do {
        bridge = next;

        ret = ...xxx...(...);
        if (ret)
            goto cleanup;

        next = bridge->bus ? bridge->bus->self : NULL;
    } while (next);
...

-- 
With Best Regards,
Andy Shevchenko