[PATCH v4 08/32] cxlflash: Fix to avoid CXL services during EEH

[Daniel Axtens]

You have two versions of check_state() below, which is a bit
confusing. It looks like you've moved the function and also added the
up/down of the read semaphore. I assume that's all that changed?

>  
>  /**
> + * check_state() - checks and responds to the current adapter state
> + * @cfg:	Internal structure associated with the host.
> + *
> + * This routine can block and should only be used on process context.
> + * It assumes that the caller is an ioctl thread and holding the ioctl
> + * read semaphore. This is temporarily let up across the wait to allow
> + * for draining actively running ioctls. Also note that when waking up
> + * from waiting in reset, the state is unknown and must be checked again
> + * before proceeding.
> + *
> + * Return: 0 on success, -errno on failure
> + */
> +static int check_state(struct cxlflash_cfg *cfg)
> +{
> +	struct device *dev = &cfg->dev->dev;
> +	int rc = 0;
> +
> +retry:
> +	switch (cfg->state) {
> +	case STATE_LIMBO:
> +		dev_dbg(dev, "%s: Limbo state, going to wait...\n", __func__);
> +		up_read(&cfg->ioctl_rwsem);
> +		rc = wait_event_interruptible(cfg->limbo_waitq,
> +					      cfg->state != STATE_LIMBO);
> +		down_read(&cfg->ioctl_rwsem);
> +		if (unlikely(rc))
> +			break;
> +		goto retry;
> +	case STATE_FAILTERM:
> +		dev_dbg(dev, "%s: Failed/Terminating!\n", __func__);
> +		rc = -ENODEV;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return rc;
> +}
> +
> +/**
>   * cxlflash_disk_attach() - attach a LUN to a context
>   * @sdev:	SCSI device associated with LUN.
>   * @attach:	Attach ioctl data structure.
> @@ -1523,41 +1563,6 @@ err1:
>  }
>  
>  /**
> - * check_state() - checks and responds to the current adapter state
> - * @cfg:	Internal structure associated with the host.
> - *
> - * This routine can block and should only be used on process context.
> - * Note that when waking up from waiting in limbo, the state is unknown
> - * and must be checked again before proceeding.
> - *
> - * Return: 0 on success, -errno on failure
> - */
> -static int check_state(struct cxlflash_cfg *cfg)
> -{
> -	struct device *dev = &cfg->dev->dev;
> -	int rc = 0;
> -
> -retry:
> -	switch (cfg->state) {
> -	case STATE_LIMBO:
> -		dev_dbg(dev, "%s: Limbo, going to wait...\n", __func__);
> -		rc = wait_event_interruptible(cfg->limbo_waitq,
> -					      cfg->state != STATE_LIMBO);
> -		if (unlikely(rc))
> -			break;
> -		goto retry;
> -	case STATE_FAILTERM:
> -		dev_dbg(dev, "%s: Failed/Terminating!\n", __func__);
> -		rc = -ENODEV;
> -		break;
> -	default:
> -		break;
> -	}
> -
> -	return rc;
> -}
> -
> -/**
>   * cxlflash_afu_recover() - initiates AFU recovery
>   * @sdev:	SCSI device associated with LUN.
>   * @recover:	Recover ioctl data structure.
> @@ -1646,9 +1651,14 @@ retry_recover:
>  	/* Test if in error state */
>  	reg = readq_be(&afu->ctrl_map->mbox_r);
>  	if (reg == -1) {
> -		dev_dbg(dev, "%s: MMIO read fail! Wait for recovery...\n",
> -			__func__);
> -		mutex_unlock(&ctxi->mutex);
> +		dev_dbg(dev, "%s: MMIO fail, wait for recovery.\n", __func__);
> +
> +		/*
> +		 * Before checking the state, put back the context obtained with
> +		 * get_context() as it is no longer needed and sleep for a short
> +		 * period of time (see prolog notes).
> +		 */
> +		put_context(ctxi);

Is this needed for the drain to work? It looks like it fixes a
refcounting bug in the function, but I'm not sure I understand how it
interacts with the rest of this patch.

Anyway, the patch overall looks good to me, and makes your driver
interact with CXL's EEH support in the way I intended when I wrote it.

Reviewed-by: Daniel Axtens <dja at axtens.net>

Regards,
Daniel