fix host max depth checking for the 'queue_depth' sysfs interface
PATCH 1/1 fix host max depth checking for the 'queue_depth' sysfs interface
Commit 1e6f2416044c0 changed the scsi sysfs 'queue_depth' code to rejects depths higher than the scsi host template setting. But lots of hosts set this to 1, and update the settings in the scsi host when the controller/devices probing happens.
This breaks (at least) mpt2sas and mpt3sas runtime setting of queue depth, returning EINVAL for all settings but '1'. And once it's set to 1, there's no way to go back up.
--- drivers/scsi/scsi_sysfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/scsi/scsi_sysfs.c +1 -1
@@ -859,7 +859,7 @@ sdev_store_queue_depth(struct device *dev, struct device_attribute *attr, depth =simple_strtoul (buf,NULL ,0 ); -if (depth <1 || depth > sht->can_queue) +if (depth <1 || depth > sdev->host->can_queue)return -EINVAL ; retval = sht->change_queue_depth (sdev, depth);
Signature · 4 lines
2.4.1.168.g1ea28e1 -- Jens Axboe
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Jens" == Jens Axboe <[email protected]> writes:
Jens> Commit 1e6f2416044c0 changed the scsi sysfs 'queue_depth' code to Jens> rejects depths higher than the scsi host template setting. But Jens> lots of hosts set this to 1, and update the settings in the scsi Jens> host when the controller/devices probing happens.
Yeah. We can't rely on the template for this.
Signature · 1 line
Martin K. Petersen Oracle Linux Engineering
Looks good:
I wonder if we should start to gradually phase out these tuables in the host template. It's not really more complicated to set them directly in the host anyway.
Quoted 7 lines · Christoph Hellwig
Looks good:
Reviewed-by Christoph Hellwig <[email protected]>I wonder if we should start to gradually phase out these tuables in the host template. It's not really more complicated to set them directly in the host anyway.
Fully agreed. I would vote for making the host template read-only and modify all drivers to use the shost setting.
Cheers,
Hannes
Signature · 9 lines
Dr. Hannes Reinecke zSeries & Storage [email protected] +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 11 lines · Hannes Reinecke
Quoted 7 lines · Christoph Hellwig
Looks good:
Reviewed-by Christoph Hellwig <[email protected]>I wonder if we should start to gradually phase out these tuables in the host template. It's not really more complicated to set them directly in the host anyway.
Fully agreed. I would vote for making the host template read-only and modify all drivers to use the shost setting.
Indeed, that would be a much saner choice. The settings in a (effectively already) read-only host template seems like somewhat of a relic.
Signature · 1 line
Jens Axboe
Quoted 12 lines · Jens Axboe
Quoted 11 lines · Hannes Reinecke
Quoted 9 lines · Christoph Hellwig
Looks good:
Reviewed-by Christoph Hellwig <[email protected]>I wonder if we should start to gradually phase out these tuables in the
host template. It's not really more complicated to set them directly
in the host anyway.
Quoted 7 lines
Quoted 2 lines
Fully agreed. I would vote for making the host template read-only and modify all drivers to use the shost setting.
Indeed, that would be a much saner choice. The settings in a (effectively already) read-only host template seems like somewhat of a relic.
Plus some drivers have the habit of modifying the template based on the settings _for this particular_ device, which leads to interesting results if you have several devices with _different_ settings ...
Cheers,
Hannes
Signature · 9 lines
Dr. Hannes Reinecke zSeries & Storage [email protected] +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Jens" == Jens Axboe <[email protected]> writes:
Fully agreed. I would vote for making the host template read-only and modify all drivers to use the shost setting.
Jens> Indeed, that would be a much saner choice. The settings in a Jens> (effectively already) read-only host template seems like somewhat Jens> of a relic.
Agree 100%.
Signature · 1 line
Martin K. Petersen Oracle Linux Engineering
Quoted 11 lines · Martin K. Petersen
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Jens" == Jens Axboe <[email protected]> writes:
Fully agreed. I would vote for making the host template read-only and modify all drivers to use the shost setting.
Jens> Indeed, that would be a much saner choice. The settings in a Jens> (effectively already) read-only host template seems like somewhat Jens> of a relic.
Agree 100%.
While we're at it: why is the 'cmd_pool' attached to the host template? Do all hosts with the same driver share a common pool? (I sincerely do hope not ...)
Cheers,
Hannes
Signature · 9 lines
Dr. Hannes Reinecke zSeries & Storage [email protected] +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html