Re: [PATCH v3 28/32] cxlflash: Fix to avoid state change collision

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> On Sep 25, 2015, at 4:10 PM, Brian King <brking@xxxxxxxxxxxxxxxxxx> wrote:
> On 09/24/2015 02:42 PM, Matthew R. Ochs wrote:
>> 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?

You bring up some good points.

I'm going to remove this patch from the set and rework it.


-matt
--
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



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux