On 20.1.2016 16:08, Sumit Saxena wrote: >> -----Original Message----- >> From: Tomas Henzl [mailto:the...@redhat.com] >> Sent: Wednesday, January 20, 2016 7:46 PM >> To: Sumit Saxena; jbottom...@parallels.com; h...@infradead.org; >> martin.peter...@oracle.com >> Cc: linux-scsi@vger.kernel.org; Kashyap Desai >> Subject: Re: [PATCH 09/15] megaraid_sas: Dual Queue depth support >> >> On 20.1.2016 15:09, Sumit Saxena wrote: >>>> -----Original Message----- >>>> From: Tomas Henzl [mailto:the...@redhat.com] >>>> Sent: Wednesday, January 20, 2016 7:26 PM >>>> To: Sumit Saxena; jbottom...@parallels.com; h...@infradead.org; >>>> martin.peter...@oracle.com >>>> Cc: linux-scsi@vger.kernel.org; Kashyap Desai >>>> Subject: Re: [PATCH 09/15] megaraid_sas: Dual Queue depth support >>>> >>>> On 19.1.2016 14:44, Sumit Saxena wrote: >>>>>> -----Original Message----- >>>>>> From: Tomas Henzl [mailto:the...@redhat.com] >>>>>> Sent: Tuesday, January 19, 2016 7:04 PM >>>>>> To: Sumit Saxena; jbottom...@parallels.com; h...@infradead.org; >>>>>> martin.peter...@oracle.com >>>>>> Cc: linux-scsi@vger.kernel.org; kashyap.de...@avagotech.com >>>>>> Subject: Re: [PATCH 09/15] megaraid_sas: Dual Queue depth support >>>>>> >>>>>> On 18.12.2015 14:27, Sumit Saxena wrote: >>>>>>> This patch will add support for Dual Queue depth reported by >>>>>>> firmware. >>>>>>> >>>>>>> Below are key points- >>>>>>> >>>>>>> 1. For iMR controllers, firmware will report two queue depths- 1. >>>>> Controller >>>>>> wide Queue depth 2. LDIO Queue depth(240). >>>>>>> Ofcourse, Controller wide Queue depth will be greater among two. >>>>>>> Using this new method, iMR can provide larger Queue depth(QD) for >>>>>>> JBOD and limited QD for Virtual Disk(VD). This feature gives >>>>>>> benefit for iMR >>>>> product >>>>>> which will be used for deployment with large number of JBOD and >>>>>> limited number of VD on setup. >>>>>>> 2. megaraid_sas driver will throttle Read write LDIOs based when >>>>>>> RW >>>>> LDIOs >>>>>> reaches "LDIO Queue Depth". >>>>>>> 3. This feature of dual queue depth can enabled/disabled via >>>>>>> module >>>>>> parameter. Default behavior is: Dual Queue depth is enabled. >>>>>>> 4. Added sysfs parameter "ldio_outstanding" for user to read LDIO >>>>> outstanding >>>>>> at run time. >>>>>>> Signed-off-by: Sumit Saxena <sumit.sax...@avagotech.com> >>>>>>> Signed-off-by: Kashyap Desai <kashyap.de...@avagotech.com> >>>>>>> --- >>>>>>> drivers/scsi/megaraid/megaraid_sas.h | 9 +++ >>>>>>> drivers/scsi/megaraid/megaraid_sas_base.c | 20 ++++++- >>>>>>> drivers/scsi/megaraid/megaraid_sas_fusion.c | 89 >>>>>> ++++++++++++++++++++++++--- >>>>>>> 3 files changed, 108 insertions(+), 10 deletions(-) >>>>>>> >>>>>>> diff --git a/drivers/scsi/megaraid/megaraid_sas.h >>>>>>> b/drivers/scsi/megaraid/megaraid_sas.h >>>>>>> index c539516..4595ef4 100644 >>>>>>> --- a/drivers/scsi/megaraid/megaraid_sas.h >>>>>>> +++ b/drivers/scsi/megaraid/megaraid_sas.h >>>>>>> @@ -1353,6 +1353,12 @@ enum DCMD_TIMEOUT_ACTION { >>>>>>> KILL_ADAPTER = 1, >>>>>>> IGNORE_TIMEOUT = 2, >>>>>>> }; >>>>>>> + >>>>>>> +enum FW_BOOT_CONTEXT { >>>>>>> + PROBE_CONTEXT = 0, >>>>>>> + OCR_CONTEXT = 1, >>>>>>> +}; >>>>>>> + >>>>>>> /* Frame Type */ >>>>>>> #define IO_FRAME 0 >>>>>>> #define PTHRU_FRAME 1 >>>>>>> @@ -2038,6 +2044,8 @@ struct megasas_instance { >>>>>>> u16 max_fw_cmds; >>>>>>> u16 max_mfi_cmds; >>>>>>> u16 max_scsi_cmds; >>>>>>> + u16 ldio_threshold; >>>>>>> + u16 cur_can_queue; >>>>>>> u32 max_sectors_per_req; >>>>>>> struct megasas_aen_event *ev; >>>>>>> >>>>>>> @@ -2068,6 +2076,7 @@ struct megasas_instance { >>>>>>> u32 fw_support_ieee; >>>>>>> >>>>>>> atomic_t fw_outstanding; >>>>>>> + atomic_t ldio_outstanding; >>>>>>> atomic_t fw_reset_no_pci_access; >>>>>>> >>>>>>> struct megasas_instance_template *instancet; diff --git >>>>>>> a/drivers/scsi/megaraid/megaraid_sas_base.c >>>>>>> b/drivers/scsi/megaraid/megaraid_sas_base.c >>>>>>> index 3454c5e..edc26fb 100644 >>>>>>> --- a/drivers/scsi/megaraid/megaraid_sas_base.c >>>>>>> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c >>>>>>> @@ -96,6 +96,10 @@ int rdpq_enable = 1; module_param(rdpq_enable, >>>>>>> int, S_IRUGO); MODULE_PARM_DESC(rdpq_enable, " Allocate reply >>>>>>> queue in chunks for large queue depth enable/disbale Default: >>>>>>> disable(0)"); >>>>>>> >>>>>>> +unsigned int dual_qdepth_disable; >>>>>>> +module_param(dual_qdepth_disable, int, S_IRUGO); >>>>>>> +MODULE_PARM_DESC(dual_qdepth_disable, "Disable dual queue depth >>>>>>> +feature. Default: 0"); >>>>>>> + >>>>>>> MODULE_LICENSE("GPL"); >>>>>>> MODULE_VERSION(MEGASAS_VERSION); >>>>>>> MODULE_AUTHOR("megaraidlinux....@avagotech.com"); >>>>>>> @@ -1977,7 +1981,7 @@ >>>> megasas_check_and_restore_queue_depth(struct >>>>>> megasas_instance *instance) >>>>>>> spin_lock_irqsave(instance->host->host_lock, flags); >>>>>>> instance->flag &= ~MEGASAS_FW_BUSY; >>>>>>> >>>>>>> - instance->host->can_queue = instance->max_scsi_cmds; >>>>>>> + instance->host->can_queue = instance->cur_can_queue; >>>>>>> spin_unlock_irqrestore(instance->host->host_lock, >>>>>>> flags); >>>>>>> } >>>>>>> } >>>>>>> @@ -2942,6 +2946,16 @@ megasas_page_size_show(struct device >> *cdev, >>>>>>> return snprintf(buf, PAGE_SIZE, "%ld\n", (unsigned >>>>>>> long)PAGE_SIZE >>>>> - >>>>>>> 1); } >>>>>>> >>>>>>> +static ssize_t >>>>>>> +megasas_ldio_outstanding_show(struct device *cdev, struct >>>>> device_attribute >>>>>> *attr, >>>>>>> + char *buf) >>>>>>> +{ >>>>>>> + struct Scsi_Host *shost = class_to_shost(cdev); >>>>>>> + struct megasas_instance *instance = (struct megasas_instance >>>>>>> +*)shost->hostdata; >>>>>>> + >>>>>>> + return snprintf(buf, PAGE_SIZE, "%d\n", >>>>>>> +atomic_read(&instance->ldio_outstanding)); >>>>>>> +} >>>>>>> + >>>>>>> static DEVICE_ATTR(fw_crash_buffer, S_IRUGO | S_IWUSR, >>>>>>> megasas_fw_crash_buffer_show, megasas_fw_crash_buffer_store); >>>>>>> static DEVICE_ATTR(fw_crash_buffer_size, S_IRUGO, @@ -2950,12 >>>>>>> +2964,15 @@ static DEVICE_ATTR(fw_crash_state, S_IRUGO | S_IWUSR, >>>>>>> megasas_fw_crash_state_show, megasas_fw_crash_state_store); >>>>>> static >>>>>>> DEVICE_ATTR(page_size, S_IRUGO, >>>>>>> megasas_page_size_show, NULL); >>>>>>> +static DEVICE_ATTR(ldio_outstanding, S_IRUGO, >>>>>>> + megasas_ldio_outstanding_show, NULL); >>>>>>> >>>>>>> struct device_attribute *megaraid_host_attrs[] = { >>>>>>> &dev_attr_fw_crash_buffer_size, >>>>>>> &dev_attr_fw_crash_buffer, >>>>>>> &dev_attr_fw_crash_state, >>>>>>> &dev_attr_page_size, >>>>>>> + &dev_attr_ldio_outstanding, >>>>>>> NULL, >>>>>>> }; >>>>>>> >>>>>>> @@ -4750,6 +4767,7 @@ megasas_init_adapter_mfi(struct >>>>>>> megasas_instance >>>>>> *instance) >>>>>>> sema_init(&instance->ioctl_sem, >>>>>> (MEGASAS_MFI_IOCTL_CMDS)); >>>>>>> } >>>>>>> >>>>>>> + instance->cur_can_queue = instance->max_scsi_cmds; >>>>>>> /* >>>>>>> * Create a pool of commands >>>>>>> */ >>>>>>> diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c >>>>>>> b/drivers/scsi/megaraid/megaraid_sas_fusion.c >>>>>>> index 9ad779d..7cc7806 100644 >>>>>>> --- a/drivers/scsi/megaraid/megaraid_sas_fusion.c >>>>>>> +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c >>>>>>> @@ -92,6 +92,7 @@ void megasas_start_timer(struct megasas_instance >>>>>> *instance, >>>>>>> void *fn, unsigned long interval); extern >>>>>>> struct >>>>>>> megasas_mgmt_info megasas_mgmt_info; extern int resetwaittime; >>>>>>> +extern unsigned int dual_qdepth_disable; >>>>>>> static void megasas_free_rdpq_fusion(struct megasas_instance >>>>>>> *instance); static void megasas_free_reply_fusion(struct >>>>>>> megasas_instance *instance); >>>>>>> >>>>>>> @@ -208,6 +209,67 @@ megasas_fire_cmd_fusion(struct >>>> megasas_instance >>>>>>> *instance, } >>>>>>> >>>>>>> /** >>>>>>> + * megasas_fusion_update_can_queue - Do all Adapter Queue depth >>>>>> related calculations here >>>>>>> + * @instance: >>>>>> Adapter soft state >>>>>>> + * fw_boot_context: >>>>> Whether this >>>>>> function called during probe or after OCR >>>>>>> + * >>>>>>> + * This function is only for fusion controllers. >>>>>>> + * Update host can queue, if firmware downgrade max supported >>>>> firmware >>>>>> commands. >>>>>>> + * Firmware upgrade case will be skiped because underlying >>>>>>> +firmware has >>>>>>> + * more resource than exposed to the OS. >>>>>>> + * >>>>>>> + */ >>>>>>> +static void >>>>>>> +megasas_fusion_update_can_queue(struct megasas_instance >>>>>>> +*instance, int fw_boot_context) { >>>>>>> + u16 cur_max_fw_cmds = 0; >>>>>>> + u16 ldio_threshold = 0; >>>>>>> + struct megasas_register_set __iomem *reg_set; >>>>>>> + >>>>>>> + reg_set = instance->reg_set; >>>>>>> + >>>>>>> + cur_max_fw_cmds = readl(&instance->reg_set- >>>>>>> outbound_scratch_pad_3) >>>>>>> +& 0x00FFFF; >>>>>>> + >>>>>>> + if (dual_qdepth_disable || !cur_max_fw_cmds) >>>>>>> + cur_max_fw_cmds = instance->instancet- >>>>>>> read_fw_status_reg(reg_set) & 0x00FFFF; >>>>>>> + else >>>>>>> + ldio_threshold = >>>>>>> + >>>>>>> (instance->instancet->read_fw_status_reg(reg_set) >>>>> & >>>>>> 0x00FFFF) - >>>>>>> +MEGASAS_FUSION_IOCTL_CMDS; >>>>>>> + >>>>>>> + dev_info(&instance->pdev->dev, >>>>>>> + "Current firmware maximum commands: %d\t LDIO >>>>>> thershold: %d\n", >>>>>> >>>>>> a typo in "thershold" >>>>>> >>>>>>> + cur_max_fw_cmds, ldio_threshold); >>>>>>> + >>>>>>> + if (fw_boot_context == OCR_CONTEXT) { >>>>>>> + cur_max_fw_cmds = cur_max_fw_cmds - 1; >>>>>>> + if (cur_max_fw_cmds <= instance->max_fw_cmds) { >>>>>> probably '<' instead of '<=" could be here ? >>>>>> >>>>>>> + instance->cur_can_queue = >>>>>>> + cur_max_fw_cmds - >>>>>> (MEGASAS_FUSION_INTERNAL_CMDS + >>>>>>> + >>>>>> MEGASAS_FUSION_IOCTL_CMDS); >>>>>>> + instance->host->can_queue = instance- >>>>>>> cur_can_queue; >>>>>>> + instance->ldio_threshold = ldio_threshold; >>>>>>> + } >>>>>>> + } else { >>>>>>> + instance->max_fw_cmds = cur_max_fw_cmds; >>>>>>> + instance->ldio_threshold = ldio_threshold; >>>>>>> + >>>>>>> + if (!instance->is_rdpq) >>>>>>> + instance->max_fw_cmds = min_t(u16, instance- >>>>>>> max_fw_cmds, 1024); >>>>>>> + >>>>>>> + /* >>>>>>> + * Reduce the max supported cmds by 1. This is to ensure >>>>> that >>>>>> the >>>>>>> + * reply_q_sz (1 more than the max cmd that driver may >>>>> send) >>>>>>> + * does not exceed max cmds that the FW can support >>>>>>> + */ >>>>>>> + instance->max_fw_cmds = instance->max_fw_cmds-1; >>>>>>> + >>>>>>> + instance->max_scsi_cmds = instance->max_fw_cmds - >>>>>>> + (MEGASAS_FUSION_INTERNAL_CMDS + >>>>>>> + MEGASAS_FUSION_IOCTL_CMDS); >>>>>>> + instance->cur_can_queue = instance->max_scsi_cmds; >>>>>>> + } >>>>>>> +} >>>>>>> +/** >>>>>>> * megasas_free_cmds_fusion - Free all the cmds in the free cmd >>>>> pool >>>>>>> * @instance: Adapter soft state >>>>>>> */ >>>>>>> @@ -736,6 +798,8 @@ megasas_ioc_init_fusion(struct >>>>>>> megasas_instance >>>>>> *instance) >>>>>>> drv_ops->mfi_capabilities.support_ext_io_size = 1; >>>>>>> >>>>>>> drv_ops->mfi_capabilities.support_fp_rlbypass = 1; >>>>>>> + if (!dual_qdepth_disable) >>>>>>> + drv_ops->mfi_capabilities.support_ext_queue_depth = 1; >>>>>>> >>>>>>> /* Convert capability to LE32 */ >>>>>>> cpu_to_le32s((u32 >>>>>>> *)&init_frame->driver_operations.mfi_capabilities); >>>>>>> @@ -1151,15 +1215,7 @@ megasas_init_adapter_fusion(struct >>>>>>> megasas_instance *instance) >>>>>>> >>>>>>> reg_set = instance->reg_set; >>>>>>> >>>>>>> - /* >>>>>>> - * Get various operational parameters from status register >>>>>>> - */ >>>>>>> - instance->max_fw_cmds = >>>>>>> - instance->instancet->read_fw_status_reg(reg_set) & >>>>> 0x00FFFF; >>>>>>> - dev_info(&instance->pdev->dev, >>>>>>> - "firmware support max fw cmd\t: (%d)\n", instance- >>>>>>> max_fw_cmds); >>>>>>> - if (!instance->is_rdpq) >>>>>>> - instance->max_fw_cmds = min_t(u16, instance- >>>>>>> max_fw_cmds, 1024); >>>>>>> + megasas_fusion_update_can_queue(instance, PROBE_CONTEXT); >>>>>>> >>>>>>> /* >>>>>>> * Reduce the max supported cmds by 1. This is to ensure that >>>>>>> the >>>>> @@ >>>>>>> -2117,6 +2173,15 @@ megasas_build_and_issue_cmd_fusion(struct >>>>>>> megasas_instance *instance, >>>>>>> >>>>>>> fusion = instance->ctrl_context; >>>>>>> >>>>>>> + if (megasas_cmd_type(scmd) == READ_WRITE_LDIO) { >>>>>>> + if (instance->ldio_threshold && >>>>>>> + (atomic_read(&instance->ldio_outstanding) >= >>>>>>> + instance->ldio_threshold)) >>>>>> This test above won't you protect when several processes read the >>>>>> same >>>>> value >>>>>> in parallel, so it may happen that you get over the limit set for >>>>> ldio_threshold. >>>>>> (You might use instead a construction with atomic_dec_and_test for >>>>> example) >>>>> >>>>> Agree..I will fix this and send updated patch. >>>> In addition to my previous comments - I'm no sure if the idea of two >>>> queues doesn't have some pitfalls - your parallel queue is based on >>>> returning commands to the midlayer with SCSI_MLQUEUE_DEVICE_BUSY - >>>> that makes it repeatedly post it to your queue again. Isn't there a >>>> performance loss with Virtual Disks ? >>> Yes this is already covered internally, there would be perf penalty >>> with VDs in configuration but this feature will be turned on based on >>> firmware settings and that specific firmware deployment has primary >>> purpose of increasing JBOD performance. This focuses on firmware >>> deployment with less VDs(or no) and more JBODs in configuration. >> OK, so it is switched off by default in the firmware and the user and an >> educated >> user can switch it on ? if so, it's fine for me. > There are different flavors of firmware using same driver and for specific > firmware this will be turned on(by default) where it's really needed. > For rest of firmware flavors, this will be switched off(not supported).
Please add this information to the patch when you will resend it. >>> Thanks, >>> Sumit >>> >>>>>> tomash > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
