>-----Original Message----- >From: Ric Wheeler [mailto:ricwheeler@xxxxxxxxx] >Sent: Tuesday, October 18, 2016 6:38 PM >To: Tomas Henzl; Sumit Saxena; linux-scsi@xxxxxxxxxxxxxxx >Cc: martin.petersen@xxxxxxxxxx; jejb@xxxxxxxxxxxxxxxxxx; Kashyap Desai >Subject: Re: [PATCH 4/7] megaraid_sas: Send SYNCHRONIZE_CACHE command to >firmware > >On 10/17/2016 09:57 AM, Tomas Henzl wrote: >> On 17.10.2016 15:28, Sumit Saxena wrote: >>>> -----Original Message----- >>>> From: Tomas Henzl [mailto:thenzl@xxxxxxxxxx] >>>> Sent: Monday, October 17, 2016 6:44 PM >>>> To: Sumit Saxena; linux-scsi@xxxxxxxxxxxxxxx >>>> Cc: martin.petersen@xxxxxxxxxx; jejb@xxxxxxxxxxxxxxxxxx; >>>> kashyap.desai@xxxxxxxxxxxx >>>> Subject: Re: [PATCH 4/7] megaraid_sas: Send SYNCHRONIZE_CACHE >>>> command to firmware >>>> >>>> On 17.10.2016 12:24, Sumit Saxena wrote: >>>>> megaraid_sas driver returns SYNCHRONIZE_CACHE command back to SCSI >>>>> layer without sending it to firmware as firmware takes care of >>>>> flushing >>> cache. >>>>> This patch will change the driver behavior wrt SYNCHRONIZE_CACHE >>> handling. >>>>> If underlying firmware has support to handle the SYNCHORNIZE_CACHE, >>>>> driver will send it for firmware otherwise complete it back to SCSI >>>>> layer with SUCCESS immediately. >>>>> If Firmware handle SYNCHORNIZE_CACHE for both VD and JBOD >>>>> "canHandleSyncCache" bit in scratch pad register(offset 0x00B4) >>>>> will be set. >>>>> >>>>> This behavior can be controlled via module parameter and user can >>>>> fallback to old behavior of returning SYNCHRONIZE_CACHE by driver >>>>> only without sending it to firmware. >>>>> >>>>> Signed-off-by: Sumit Saxena <sumit.saxena@xxxxxxxxxxxx> >>>>> Signed-off-by: Kashyap Desai <kashyap.desai@xxxxxxxxxxxx> >>>>> --- >>>>> drivers/scsi/megaraid/megaraid_sas.h | 3 +++ >>>>> drivers/scsi/megaraid/megaraid_sas_base.c | 14 ++++++-------- >>>>> drivers/scsi/megaraid/megaraid_sas_fusion.c | 3 +++ >>>>> drivers/scsi/megaraid/megaraid_sas_fusion.h | 2 ++ >>>>> 4 files changed, 14 insertions(+), 8 deletions(-) >>>>> >>>>> diff --git a/drivers/scsi/megaraid/megaraid_sas.h >>>>> b/drivers/scsi/megaraid/megaraid_sas.h >>>>> index ca86c88..43fd14f 100644 >>>>> --- a/drivers/scsi/megaraid/megaraid_sas.h >>>>> +++ b/drivers/scsi/megaraid/megaraid_sas.h >>>>> @@ -1429,6 +1429,8 @@ enum FW_BOOT_CONTEXT { >>>>> #define MR_MAX_REPLY_QUEUES_EXT_OFFSET_SHIFT 14 >>>>> #define MR_MAX_MSIX_REG_ARRAY 16 >>>>> #define MR_RDPQ_MODE_OFFSET 0X00800000 >>>>> +#define MR_CAN_HANDLE_SYNC_CACHE_OFFSET 0X01000000 >>>>> + >>>>> /* >>>>> * register set for both 1068 and 1078 controllers >>>>> * structure extended for 1078 registers @@ -2140,6 +2142,7 @@ >>>>> struct megasas_instance { >>>>> u8 is_imr; >>>>> u8 is_rdpq; >>>>> bool dev_handle; >>>>> + bool fw_sync_cache_support; >>>>> }; >>>>> struct MR_LD_VF_MAP { >>>>> u32 size; >>>>> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c >>>>> b/drivers/scsi/megaraid/megaraid_sas_base.c >>>>> index 092387f..a4a8e2f 100644 >>>>> --- a/drivers/scsi/megaraid/megaraid_sas_base.c >>>>> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c >>>>> @@ -104,6 +104,10 @@ unsigned int scmd_timeout = >>>>> MEGASAS_DEFAULT_CMD_TIMEOUT; module_param(scmd_timeout, int, >>>>> S_IRUGO); MODULE_PARM_DESC(scmd_timeout, "scsi command timeout >>>>> (10-90s), default 90s. See megasas_reset_timer."); >>>>> >>>>> +bool block_sync_cache; >>>>> +module_param(block_sync_cache, bool, S_IRUGO); >>>>> +MODULE_PARM_DESC(block_sync_cache, "Block SYNC CACHE by driver >>>>> +Default: 0(Send it to firmware)"); >>>>> + >>>>> MODULE_LICENSE("GPL"); >>>>> MODULE_VERSION(MEGASAS_VERSION); >>>>> MODULE_AUTHOR("megaraidlinux.pdl@xxxxxxxxxxxxx"); >>>>> @@ -1700,16 +1704,10 @@ megasas_queue_command(struct Scsi_Host >>>> *shost, struct scsi_cmnd *scmd) >>>>> goto out_done; >>>>> } >>>>> >>>>> - switch (scmd->cmnd[0]) { >>>>> - case SYNCHRONIZE_CACHE: >>>>> - /* >>>>> - * FW takes care of flush cache on its own >>>>> - * No need to send it down >>>>> - */ >>>>> + if ((scmd->cmnd[0] == SYNCHRONIZE_CACHE) && >>>>> + (!instance->fw_sync_cache_support)) { >>>> Maybe I'm wrong, but isn't the logic inverted? >>>> when fw_sync_cache_support is true it means that there is a flash >>>> cache >>> or a >>>> battery and we don't have to send the command down ? >>>> >>> If "instance->fw_sync_cache_support" is set to true, it means driver >>> can send SYNCHRONIZE_CACHE to firmware(firmware has infrastructure to >>> support SYNCHRONIZE_CACHE). >>> If "instance->fw_sync_cache_support" is set to false, it means FW >>> does not support to handle SYNCHRONIZE_CACHE and FW will flush cache >>> during shutdown. >> Thanks, in that case it is correct. >> >>>>> + if (!block_sync_cache) >>>>> + instance->fw_sync_cache_support = (scratch_pad_2 & >>>>> + MR_CAN_HANDLE_SYNC_CACHE_OFFSET) ? 1 : 0; >> Please add a warning or log the state of the synchronise cache command >> on the controller. > >Any logging should be limited to once per device - say when the device is >opened. >Note that synchronize_cache commands are extremely common, we don't want >to fill the log with this. This print is not per device but per MegaRAID controller. This print will state the controller firmware's capability whether SYNCHRONIZE_CACHE coming from OS for RAID volumes can be handled by firmware or not. For non RAID(JBODs), SYNCHRONIZE_CACHE will be always sent to all MegaRAID firmware irrespective of firmware capability. Thanks, Sumit > >thanks! > >Ric > >> >> >>>>> IOCInitMessage = >>>>> dma_alloc_coherent(&instance->pdev->dev, >>>>> diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.h >>>>> b/drivers/scsi/megaraid/megaraid_sas_fusion.h >>>>> index e3bee04..2857154 100644 >>>>> --- a/drivers/scsi/megaraid/megaraid_sas_fusion.h >>>>> +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.h >>>>> @@ -72,6 +72,8 @@ >>>>> #define MPI2_SUP_REPLY_POST_HOST_INDEX_OFFSET (0x0000030C) >>>>> #define MPI2_REPLY_POST_HOST_INDEX_OFFSET (0x0000006C) >>>>> >>>>> +extern bool block_sync_cache; >>>>> + >>>>> /* >>>>> * Raid context flags >>>>> */ -- 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
