Subject: Re: [PATCH net 5/5] ibmvnic: Ensure login failure recovery is safe from other resets

From: Jakub Kicinski <[email-masked]>
Date: Mon, 7 Aug 2023 19:13:39 -0700

On Thu,  3 Aug 2023 15:20:10 -0500 Nick Child wrote:
> +			do {
> +				reinit_init_done(adapter);
> +				/* Clear any failovers we got in the previous
> +				 * pass since we are re-initializing the CRQ
> +				 */
> +				adapter->failover_pending = false;
> +				release_crq_queue(adapter);
> +				/* If we don't sleep here then we risk an
> +				 * unnecessary failover event from the VIOS.
> +				 * This is a known VIOS issue caused by a vnic
> +				 * device freeing and registering a CRQ too
> +				 * quickly.
> +				 */
> +				msleep(1500);
> +				/* Avoid any resets, since we are currently
> +				 * resetting.
> +				 */
> +				spin_lock_irqsave(&adapter->rwi_lock, flags);
> +				flush_reset_queue(adapter);
> +				spin_unlock_irqrestore(&adapter->rwi_lock,
> +						       flags);
> +
> +				rc = init_crq_queue(adapter);
> +				if (rc) {
> +					netdev_err(netdev, "login recovery: init CRQ failed %d\n",
> +						   rc);
> +					return -EIO;
> +				}
>  
> -			rc = ibmvnic_reset_init(adapter, false);
> -			if (rc) {
> -				netdev_err(netdev, "login recovery: Reset init failed %d\n",
> -					   rc);
> -				return -EIO;
> -			}
> +				rc = ibmvnic_reset_init(adapter, false);
> +				if (rc)
> +					netdev_err(netdev, "login recovery: Reset init failed %d\n",
> +						   rc);
> +				/* IBMVNIC_CRQ_INIT will return EAGAIN if it
> +				 * fails, since ibmvnic_reset_init will free
> +				 * irq's in failure, we won't be able to receive
> +				 * new CRQs so we need to keep trying. probe()
> +				 * handles this similarly.
> +				 */
> +			} while (rc == -EAGAIN);

Isn't this potentially an infinite loop? Can we limit the max number of
iterations here or something already makes this loop safe?