Re: [PATCH 09/30] scsi: kill off the legacy IO path

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

 



On 11/1/18 3:11 PM, Omar Sandoval wrote:
>> -		if ((ql2x_ini_mode == QLA2XXX_INI_MODE_ENABLED) &&
>> -		    shost_use_blk_mq(host)) {
>> -			mq = true;
>> +		if (ql2x_ini_mode == QLA2XXX_INI_MODE_ENABLED)
>>  			startit = true;
>> -		}
> 
> This could just be
> 
> 		startit = (QLA_TGT_MODE_ENABLED() ||
> 			   (ql2x_ini_mode == QLA2XXX_INI_MODE_ENABLED));

I agree, just didn't want to make changes that would be harder to
verify. So kept as much as the original style as possible.

>> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
>> index fc1356d101b0..99db3f4316b5 100644
>> --- a/drivers/scsi/scsi.c
>> +++ b/drivers/scsi/scsi.c
>> @@ -780,11 +780,8 @@ MODULE_LICENSE("GPL");
>>  module_param(scsi_logging_level, int, S_IRUGO|S_IWUSR);
>>  MODULE_PARM_DESC(scsi_logging_level, "a bit mask of logging levels");
>>  
>> -#ifdef CONFIG_SCSI_MQ_DEFAULT
>> +/* Kill module parameter */
> 
> Is this a leftover todo comment for yourself, or a note for the future?
> If the latter, I think it could be clearer.

It's just a note for the future. I'll improve it.

>> -		unsigned long flags;
>> -
>> -		if (bidi_bytes)
>> -			scsi_release_bidi_buffers(cmd);
>> -		scsi_release_buffers(cmd);
>> -		scsi_put_command(cmd);
>> +	/*
>> +	 * In the MQ case the command gets freed by __blk_mq_end_request,
>> +	 * so we have to do all cleanup that depends on it earlier.
>> +	 *
>> +	 * We also can't kick the queues from irq context, so we
>> +	 * will have to defer it to a workqueue.
>> +	 */
> 
> This comment is slightly stale, since everything is the MQ case now.

Agree, but it's just copied over. Not a huge deal since it's generally
applicable, there's just no alternative :-)

>> @@ -367,7 +367,6 @@ store_shost_eh_deadline(struct device *dev, struct device_attribute *attr,
>>  
>>  static DEVICE_ATTR(eh_deadline, S_IRUGO | S_IWUSR, show_shost_eh_deadline, store_shost_eh_deadline);
>>  
>> -shost_rd_attr(use_blk_mq, "%d\n");
>>  shost_rd_attr(unique_id, "%u\n");
>>  shost_rd_attr(cmd_per_lun, "%hd\n");
>>  shost_rd_attr(can_queue, "%hd\n");
>> @@ -386,6 +385,13 @@ show_host_busy(struct device *dev, struct device_attribute *attr, char *buf)
>>  }
>>  static DEVICE_ATTR(host_busy, S_IRUGO, show_host_busy, NULL);
>>  
>> +static ssize_t
>> +show_use_blk_mq(struct device *dev, struct device_attribute *attr, char *buf)
>> +{
>> +	return snprintf(buf, 20, "1\n");
>> +}
> 
> Looks like you forgot to change this to sprintf()

Indeed, I'll make that change.

Thanks for the review!

-- 
Jens Axboe




[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux