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@xxxxxxxxxx>
Regards,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
