Re: [PATCH] megaraid_sas: Enable shared tag map

Christoph Hellwig
2014-09-30T07:43:31Z

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?

>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
> 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?

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