Re: [PATCH net 3/4] ibmvnic: Bound waits for device queries

[Jakub Kicinski]

On Fri, 22 Nov 2019 13:41:45 -0600, Thomas Falcon wrote:
> +static int ibmvnic_wait_for_completion(struct ibmvnic_adapter *adapter,
> +                                    struct completion *comp_done,
> +                                    unsigned long timeout)
> +{
> +     struct net_device *netdev = adapter->netdev;
> +     u8 retry = 5;
> +
> +restart_timer:
> +     if (!adapter->crq.active) {
> +             netdev_err(netdev, "Device down!\n");
> +             return -ENODEV;
> +     }
> +     /* periodically check that the device is up while waiting for
> +      * a response
> +      */
> +     if (!wait_for_completion_timeout(comp_done, timeout / retry)) {
> +             if (!adapter->crq.active) {
> +                     netdev_err(netdev, "Device down!\n");
> +                     return -ENODEV;
> +             } else {
> +                     retry--;
> +                     if (retry)
> +                             goto restart_timer;
> +                     netdev_err(netdev, "Operation timing out...\n");
> +                     return -ETIMEDOUT;

Hm. This is not great. I don't see the need to open code a loop with
a goto:

while (true) {
        if (down())
                return E;

        if (retry--)
                break;

        if (wait())
                return 0
}

print(time out);
return E;

The wait_for_completion_timeout() will not be very precise, but I think
with 5 sleeps it shouldn't drift off too far from the desired 10sec.

> +             }
> +     }
> +
> +     return 0;
> +}