Re: [PATCH v3 28/32] cxlflash: Fix to avoid state change collision
From: Brian King <hidden>
Date: 2015-09-25 21:10:39
Also in:
linux-scsi
On 09/24/2015 02:42 PM, Matthew R. Ochs wrote:
quoted hunk ↗ jump to hunk
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c index ab11ee6..325ba31 100644 --- a/drivers/scsi/cxlflash/main.c +++ b/drivers/scsi/cxlflash/main.c@@ -496,6 +496,7 @@ static int cxlflash_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scp) struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)host->hostdata; struct afu *afu = cfg->afu; struct device *dev = &cfg->dev->dev; + enum cxlflash_state state; struct afu_cmd *cmd; u32 port_sel = scp->device->channel + 1; int nseg, i, ncount;@@ -525,7 +526,11 @@ static int cxlflash_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scp) } spin_unlock_irqrestore(&cfg->tmf_slock, lock_flags); - switch (cfg->state) { + mutex_lock(&cfg->mutex); + state = cfg->state; + mutex_unlock(&cfg->mutex);
You can't grab a mutex in queuecommand, since it can sleep and queuecommand can be called from soft irq context. Also, I'm not sure what to think about this patch in general. Obviously state can change immediately after you drop the mutex, and according to Documentation/memory-barriers.txt, memory operations after the unlock can occur before the unlock occurs. Is this a problem?
+
+ switch (state) {
case STATE_RESET:
dev_dbg_ratelimited(dev, "%s: device is in reset!\n", __func__);
rc = SCSI_MLQUEUE_HOST_BUSY;-- Brian King Power Linux I/O IBM Linux Technology Center