On 09/30/2014 09:43 AM, Christoph Hellwig wrote:
> On Mon, Sep 29, 2014 at 01:47:52PM +0200, Hannes Reinecke wrote:
>> Megaraid_sas uses a shared pool of commands per HBA, so we
>> should be enabling a shared tag map.
>> This will allow the I/O scheduler to make better scheduling
>> decisions and will avoid BUSY states in the driver.
>
> What exact problem did you see? Do you have a link to a bugzilla entry
> or similar? Was this observed on real hardware or your qemu emulation?
>
Well, _actually_ I just did it so that I could get the tag number
for my printk patchset :-)
But there is an underlying reason:
megaraid_sas uses an internal frame pool of a fixed size.
Once that's exhausted it cannot queue any more commands, and need to
return busy.
But as this is a per-host command pool all LUNs have to shared the
same frame pool, and the driver uses heuristics (ie 1/4 of the pool
size) to set cmd_per_lun. So if you have more than 4 LUNs you'll run
into issues as the pool can become exhausted.
Hence I thought it would be good to expose this to the block layer
so that we can avoid BUSY states in the driver.
>>
>> Signed-off-by: Hannes Reinecke <hare@xxxxxxx>
>> ---
>> drivers/scsi/megaraid/megaraid_sas_base.c | 34 ++++++++++++++++++++++++++++++-
>> 1 file changed, 33 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
>> index f6a69a3..996fa9a 100644
>> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
>> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
>> @@ -1659,7 +1659,7 @@ static int megasas_slave_configure(struct scsi_device *sdev)
>> */
>> blk_queue_rq_timeout(sdev->request_queue,
>> MEGASAS_DEFAULT_CMD_TIMEOUT * HZ);
>> -
>> + sdev->tagged_supported = 1;
>
>> + /* We have a shared tag map, so TCQ is always supported */
>> + sdev->tagged_supported = 1;
>> +
>
> Why doesn't the device return the proper data in the INQUIRY response?
>
> And more importantly why do you want to do this twice?
>
Bah, Typo.
>> instance = megasas_lookup_instance(sdev->host->host_no);
>> if (sdev->channel < MEGASAS_MAX_PD_CHANNELS) {
>> /*
>> @@ -1677,13 +1681,20 @@ static int megasas_slave_alloc(struct scsi_device *sdev)
>> sdev->id;
>> if (instance->pd_list[pd_index].driveState ==
>> MR_PD_STATE_SYSTEM) {
>> + scsi_activate_tcq(sdev, MEGASAS_DEFAULT_CMD_PER_LUN);
>> return 0;
>> }
>> return -ENXIO;
>> }
>> + scsi_activate_tcq(sdev, MEGASAS_DEFAULT_CMD_PER_LUN);
>> return 0;
>
> Please refactor the code so that the first case falls through to the
> second, something like:
>
>
> if (instance->pd_list[pd_index].driveState !=
> MR_PD_STATE_SYSTEM)
> return -ENXIO;
> }
>
> scsi_activate_tcq(sdev, MEGASAS_DEFAULT_CMD_PER_LUN);
> return 0;
>
Okay, will be doing it.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@xxxxxxx +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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
