Re: [PATCH RFC v7 10/12] megaraid_sas: switch fusion adapters to MQ

John Garry
2020-07-07T07:59:50Z

>>>
>>> #include <scsi/scsi.h>
>>> #include <scsi/scsi_cmnd.h>
>>> @@ -113,6 +114,10 @@ unsigned int enable_sdev_max_qd;
>>> module_param(enable_sdev_max_qd, int, 0444);
>>> MODULE_PARM_DESC(enable_sdev_max_qd, "Enable sdev max qd as
>> can_queue.
>>> Default: 0");
>>>
>>> +int host_tagset_disabled = 0;
>>> +module_param(host_tagset_disabled, int, 0444);
>>> +MODULE_PARM_DESC(host_tagset_disabled, "Shared host tagset
>> enable/disable
>>> Default: enable(1)");
>> The logic seems inverted here: for passing 1, I would expect Shared host
>> tagset enabled, while it actually means to disable, right?
> No. passing 1 means shared_hosttag support will be turned off.

Just reading "Shared host tagset enable/disable Default: enable(1)"
looked inconsistent to me.

>>> +
>>> MODULE_LICENSE("GPL");
>>> MODULE_VERSION(MEGASAS_VERSION);
>>> MODULE_AUTHOR("megaraidlinux.pdl@broadcom.com");
>>> @@ -3115,6 +3120,18 @@ megasas_bios_param(struct scsi_device *sdev,
>> struct
>>> block_device *bdev,
>>> return 0;
>>> }
>>>
>>> +static int megasas_map_queues(struct Scsi_Host *shost)
>>> +{
>>> + struct megasas_instance *instance;
>>> + instance = (struct megasas_instance *)shost->hostdata;
>>> +
>>> + if (instance->host->nr_hw_queues == 1)
>>> + return 0;
>>> +
>>> + return
>>> blk_mq_pci_map_queues(&shost->tag_set.map[HCTX_TYPE_DEFAULT],
>>> + instance->pdev,
>>> instance->low_latency_index_start);
>>> +}
>>> +
>>> static void megasas_aen_polling(struct work_struct *work);
>>>
>>> /**
>>> @@ -3423,8 +3440,10 @@ static struct scsi_host_template
>> megasas_template =
>>> {
>>> .eh_timed_out = megasas_reset_timer,
>>> .shost_attrs = megaraid_host_attrs,
>>> .bios_param = megasas_bios_param,
>>> + .map_queues = megasas_map_queues,
>>> .change_queue_depth = scsi_change_queue_depth,
>>> .max_segment_size = 0xffffffff,
>>> + .host_tagset = 1,
>> Is your intention to always have this set for Scsi_Host, and just change
>> nr_hw_queues?
> Actually I wanted to turn off this feature using host_tagset and not
> through nr_hw_queue. I will address this.
>
> Additional request -
> In MR we have old controllers (called MFI_SERIES). We prefer not to change
> behavior for those controller.
> Having host_tagset in template does not allow to cherry pick different
> values for different type of controller.

Ok, so it seems sensible to add host_tagset to Scsi_Host structure also,
to allow overwriting during probe time.

If you want to share an updated megaraid sas driver patch based on that,
then that's fine. I can incorporate that change in the patch where we
add host_tagset to the scsi host template.

> If host_tagset is part of Scsi_Host OR we add check in scsi_lib.c that
> host_tagset = 1 only make sense if nr_hw_queues > 1, we can cherry pick in
> driver.
>
>