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:the...@redhat.com]
Sent: Monday, October 17, 2016 6:44 PM
To: Sumit Saxena; linux-scsi@vger.kernel.org
Cc: martin.peter...@oracle.com; j...@linux.vnet.ibm.com;
kashyap.de...@broadcom.com
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.sax...@broadcom.com>
Signed-off-by: Kashyap Desai <kashyap.de...@broadcom.com>
---
  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....@avagotech.com");
@@ -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.

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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to