Re: [PATCH v2 03/39] megaraid_sas: raid 1 fast path code optimize

2017-02-09 Thread Tomas Henzl
On 8.2.2017 19:51, Kashyap Desai wrote:
>>> +static inline void
>>> +megasas_complete_r1_command(struct megasas_instance *instance,
>>> +   struct megasas_cmd_fusion *cmd) {
>>> +   u8 *sense, status, ex_status;
>>> +   u32 data_length;
>>> +   u16 peer_smid;
>>> +   struct fusion_context *fusion;
>>> +   struct megasas_cmd_fusion *r1_cmd = NULL;
>>> +   struct scsi_cmnd *scmd_local = NULL;
>>> +   struct RAID_CONTEXT_G35 *rctx_g35;
>>> +
>>> +   rctx_g35 = >io_request->RaidContext.raid_context_g35;
>>> +   fusion = instance->ctrl_context;
>>> +   peer_smid = le16_to_cpu(rctx_g35->smid.peer_smid);
>>> +
>>> +   r1_cmd = fusion->cmd_list[peer_smid - 1];
>>> +   scmd_local = cmd->scmd;
>>> +   status = rctx_g35->status;
>>> +   ex_status = rctx_g35->ex_status;
>>> +   data_length = cmd->io_request->DataLength;
>>> +   sense = cmd->sense;
>>> +
>>> +   cmd->cmd_completed = true;
>> Please help me understand how this works
>> - there are two peer commands sent to the controller
>> - both are completed and the later calls scsi_done and returns both
> r1_cmd
>> + cmd
>> - if both commands can be completed at the same time, is it possible
> that
>> the
>>   above line is executed at the same moment for both completions ?
>> How is the code  protected against a double completion when both
>> completed commands see the peer cmd_completed as set ?
>
> Tomas,  cmd and r1_cmd (part of  same Raid 1 FP) will be always completed
> on same reply queue by firmware. That is one of the key requirement here
> for raid 1 fast path.
> What you ask is possible if FW completes cmd and r1_cmd on different reply
> queue. If you notice when we clone r1_cmd, we also clone MSI-x index from
> parent command.
> So eventually, FW is aware of binding of both cmd and r1_cmd w.r.t reply
> queue index.

ok, thanks

>
> ` Kashyap
>
>>> +




Re: [PATCH v2 03/39] megaraid_sas: raid 1 fast path code optimize

2017-02-09 Thread Tomas Henzl
On 8.2.2017 10:28, Shivasharan S wrote:
> fix in v2 - ex_status and status was wrongly re-used in 
> megasas_complete_r1_command.
> discussed below -
> http://marc.info/?l=linux-scsi=148638763409385=2
>
>
> No functional change. Code refactor.
> Remove function megasas_fpio_to_ldio as we never require to convert fpio to 
> ldio because of frame unavailability.
> Grab extra frame of raid 1 write fast path before it creates first frame as 
> Fast Path.
> Removed is_raid_1_fp_write flag as raid 1 write fast path command is decided 
> using r1_alt_dev_handle only.
> Move resetting megasas_cmd_fusion fields at common function 
> megasas_return_cmd_fusion.
>
>
> Signed-off-by: Shivasharan S 
> Signed-off-by: Kashyap Desai 
> Reviewed-by: Hannes Reinecke 

Reviewed-by: Tomas Henzl 

Tomas



RE: [PATCH v2 03/39] megaraid_sas: raid 1 fast path code optimize

2017-02-08 Thread Kashyap Desai
> > +static inline void
> > +megasas_complete_r1_command(struct megasas_instance *instance,
> > +   struct megasas_cmd_fusion *cmd) {
> > +   u8 *sense, status, ex_status;
> > +   u32 data_length;
> > +   u16 peer_smid;
> > +   struct fusion_context *fusion;
> > +   struct megasas_cmd_fusion *r1_cmd = NULL;
> > +   struct scsi_cmnd *scmd_local = NULL;
> > +   struct RAID_CONTEXT_G35 *rctx_g35;
> > +
> > +   rctx_g35 = >io_request->RaidContext.raid_context_g35;
> > +   fusion = instance->ctrl_context;
> > +   peer_smid = le16_to_cpu(rctx_g35->smid.peer_smid);
> > +
> > +   r1_cmd = fusion->cmd_list[peer_smid - 1];
> > +   scmd_local = cmd->scmd;
> > +   status = rctx_g35->status;
> > +   ex_status = rctx_g35->ex_status;
> > +   data_length = cmd->io_request->DataLength;
> > +   sense = cmd->sense;
> > +
> > +   cmd->cmd_completed = true;
>
> Please help me understand how this works
> - there are two peer commands sent to the controller
> - both are completed and the later calls scsi_done and returns both
r1_cmd
> + cmd
> - if both commands can be completed at the same time, is it possible
that
> the
>   above line is executed at the same moment for both completions ?
> How is the code  protected against a double completion when both
> completed commands see the peer cmd_completed as set ?


Tomas,  cmd and r1_cmd (part of  same Raid 1 FP) will be always completed
on same reply queue by firmware. That is one of the key requirement here
for raid 1 fast path.
What you ask is possible if FW completes cmd and r1_cmd on different reply
queue. If you notice when we clone r1_cmd, we also clone MSI-x index from
parent command.
So eventually, FW is aware of binding of both cmd and r1_cmd w.r.t reply
queue index.

` Kashyap

>
> > +


Re: [PATCH v2 03/39] megaraid_sas: raid 1 fast path code optimize

2017-02-08 Thread Tomas Henzl
On 8.2.2017 10:28, Shivasharan S wrote:
> fix in v2 - ex_status and status was wrongly re-used in 
> megasas_complete_r1_command.
> discussed below -
> http://marc.info/?l=linux-scsi=148638763409385=2
>
>
> No functional change. Code refactor.
> Remove function megasas_fpio_to_ldio as we never require to convert fpio to 
> ldio because of frame unavailability.
> Grab extra frame of raid 1 write fast path before it creates first frame as 
> Fast Path.
> Removed is_raid_1_fp_write flag as raid 1 write fast path command is decided 
> using r1_alt_dev_handle only.
> Move resetting megasas_cmd_fusion fields at common function 
> megasas_return_cmd_fusion.
>
>
> Signed-off-by: Shivasharan S 
> Signed-off-by: Kashyap Desai 
> Reviewed-by: Hannes Reinecke 
> ---
>  drivers/scsi/megaraid/megaraid_sas_fp.c |  14 +-
>  drivers/scsi/megaraid/megaraid_sas_fusion.c | 349 
> +---
>  drivers/scsi/megaraid/megaraid_sas_fusion.h |   3 +-
>  3 files changed, 118 insertions(+), 248 deletions(-)
>
> diff --git a/drivers/scsi/megaraid/megaraid_sas_fp.c 
> b/drivers/scsi/megaraid/megaraid_sas_fp.c
> index f1384b0..24258af 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_fp.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_fp.c
> @@ -1338,20 +1338,8 @@ MR_BuildRaidContext(struct megasas_instance *instance,
>   ref_in_start_stripe, io_info,
>   pRAID_Context, map);
>   /* If IO on an invalid Pd, then FP is not possible.*/
> - if (io_info->devHandle == cpu_to_le16(MR_PD_INVALID))
> + if (io_info->devHandle == MR_DEVHANDLE_INVALID)
>   io_info->fpOkForIo = FALSE;
> - /* if FP possible, set the SLUD bit in
> -  *  regLockFlags for ventura
> -  */
> - else if ((instance->is_ventura) && (!isRead) &&
> - (raid->writeMode == MR_RL_WRITE_BACK_MODE) &&
> - (raid->capability.fp_cache_bypass_capable))
> - ((struct RAID_CONTEXT_G35 *) 
> pRAID_Context)->routing_flags.bits.sld = 1;
> - /* set raid 1/10 fast path write capable bit in io_info */
> - if (io_info->fpOkForIo &&
> - (io_info->r1_alt_dev_handle != MR_PD_INVALID) &&
> - (raid->level == 1) && !isRead)
> - io_info->is_raid_1_fp_write = 1;
>   return retval;
>   } else if (isRead) {
>   uint stripIdx;
> diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c 
> b/drivers/scsi/megaraid/megaraid_sas_fusion.c
> index 514c306..7516589 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
> @@ -181,7 +181,9 @@ inline void megasas_return_cmd_fusion(struct 
> megasas_instance *instance,
>   struct megasas_cmd_fusion *cmd)
>  {
>   cmd->scmd = NULL;
> - memset(cmd->io_request, 0, sizeof(struct MPI2_RAID_SCSI_IO_REQUEST));
> + memset(cmd->io_request, 0, MEGA_MPI2_RAID_DEFAULT_IO_FRAME_SIZE);
> + cmd->r1_alt_dev_handle = MR_DEVHANDLE_INVALID;
> + cmd->cmd_completed = false;
>  }
>  
>  /**
> @@ -701,7 +703,7 @@ megasas_alloc_cmds_fusion(struct megasas_instance 
> *instance)
>   memset(cmd->io_request, 0,
>  sizeof(struct MPI2_RAID_SCSI_IO_REQUEST));
>   cmd->io_request_phys_addr = io_req_base_phys + offset;
> - cmd->is_raid_1_fp_write = 0;
> + cmd->r1_alt_dev_handle = MR_DEVHANDLE_INVALID;
>   }
>  
>   if (megasas_create_sg_sense_fusion(instance))
> @@ -1984,7 +1986,7 @@ megasas_build_ldio_fusion(struct megasas_instance 
> *instance,
>   io_info.ldStartBlock = ((u64)start_lba_hi << 32) | start_lba_lo;
>   io_info.numBlocks = datalength;
>   io_info.ldTgtId = device_id;
> - io_info.r1_alt_dev_handle = MR_PD_INVALID;
> + io_info.r1_alt_dev_handle = MR_DEVHANDLE_INVALID;
>   scsi_buff_len = scsi_bufflen(scp);
>   io_request->DataLength = cpu_to_le32(scsi_buff_len);
>  
> @@ -2025,7 +2027,7 @@ megasas_build_ldio_fusion(struct megasas_instance 
> *instance,
>   io_info.isRead && io_info.ra_capable)
>   fp_possible = false;
>  
> - if (io_info.r1_alt_dev_handle != MR_PD_INVALID) {
> + if (io_info.r1_alt_dev_handle != MR_DEVHANDLE_INVALID) {
>   mrdev_priv = scp->device->hostdata;
>  
>   if (atomic_inc_return(>fw_outstanding) >
> @@ -2090,9 +2092,10 @@ megasas_build_ldio_fusion(struct megasas_instance 
> *instance,
>   } else
>   scp->SCp.Status &= ~MEGASAS_LOAD_BALANCE_FLAG;
>  
> - cmd->is_raid_1_fp_write = io_info.is_raid_1_fp_write;
> - if (io_info.is_raid_1_fp_write)
> + if (instance->is_ventura)
> 

[PATCH v2 03/39] megaraid_sas: raid 1 fast path code optimize

2017-02-08 Thread Shivasharan S
fix in v2 - ex_status and status was wrongly re-used in 
megasas_complete_r1_command.
discussed below -
http://marc.info/?l=linux-scsi=148638763409385=2


No functional change. Code refactor.
Remove function megasas_fpio_to_ldio as we never require to convert fpio to 
ldio because of frame unavailability.
Grab extra frame of raid 1 write fast path before it creates first frame as 
Fast Path.
Removed is_raid_1_fp_write flag as raid 1 write fast path command is decided 
using r1_alt_dev_handle only.
Move resetting megasas_cmd_fusion fields at common function 
megasas_return_cmd_fusion.


Signed-off-by: Shivasharan S 
Signed-off-by: Kashyap Desai 
Reviewed-by: Hannes Reinecke 
---
 drivers/scsi/megaraid/megaraid_sas_fp.c |  14 +-
 drivers/scsi/megaraid/megaraid_sas_fusion.c | 349 +---
 drivers/scsi/megaraid/megaraid_sas_fusion.h |   3 +-
 3 files changed, 118 insertions(+), 248 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas_fp.c 
b/drivers/scsi/megaraid/megaraid_sas_fp.c
index f1384b0..24258af 100644
--- a/drivers/scsi/megaraid/megaraid_sas_fp.c
+++ b/drivers/scsi/megaraid/megaraid_sas_fp.c
@@ -1338,20 +1338,8 @@ MR_BuildRaidContext(struct megasas_instance *instance,
ref_in_start_stripe, io_info,
pRAID_Context, map);
/* If IO on an invalid Pd, then FP is not possible.*/
-   if (io_info->devHandle == cpu_to_le16(MR_PD_INVALID))
+   if (io_info->devHandle == MR_DEVHANDLE_INVALID)
io_info->fpOkForIo = FALSE;
-   /* if FP possible, set the SLUD bit in
-*  regLockFlags for ventura
-*/
-   else if ((instance->is_ventura) && (!isRead) &&
-   (raid->writeMode == MR_RL_WRITE_BACK_MODE) &&
-   (raid->capability.fp_cache_bypass_capable))
-   ((struct RAID_CONTEXT_G35 *) 
pRAID_Context)->routing_flags.bits.sld = 1;
-   /* set raid 1/10 fast path write capable bit in io_info */
-   if (io_info->fpOkForIo &&
-   (io_info->r1_alt_dev_handle != MR_PD_INVALID) &&
-   (raid->level == 1) && !isRead)
-   io_info->is_raid_1_fp_write = 1;
return retval;
} else if (isRead) {
uint stripIdx;
diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c 
b/drivers/scsi/megaraid/megaraid_sas_fusion.c
index 514c306..7516589 100644
--- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
+++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
@@ -181,7 +181,9 @@ inline void megasas_return_cmd_fusion(struct 
megasas_instance *instance,
struct megasas_cmd_fusion *cmd)
 {
cmd->scmd = NULL;
-   memset(cmd->io_request, 0, sizeof(struct MPI2_RAID_SCSI_IO_REQUEST));
+   memset(cmd->io_request, 0, MEGA_MPI2_RAID_DEFAULT_IO_FRAME_SIZE);
+   cmd->r1_alt_dev_handle = MR_DEVHANDLE_INVALID;
+   cmd->cmd_completed = false;
 }
 
 /**
@@ -701,7 +703,7 @@ megasas_alloc_cmds_fusion(struct megasas_instance *instance)
memset(cmd->io_request, 0,
   sizeof(struct MPI2_RAID_SCSI_IO_REQUEST));
cmd->io_request_phys_addr = io_req_base_phys + offset;
-   cmd->is_raid_1_fp_write = 0;
+   cmd->r1_alt_dev_handle = MR_DEVHANDLE_INVALID;
}
 
if (megasas_create_sg_sense_fusion(instance))
@@ -1984,7 +1986,7 @@ megasas_build_ldio_fusion(struct megasas_instance 
*instance,
io_info.ldStartBlock = ((u64)start_lba_hi << 32) | start_lba_lo;
io_info.numBlocks = datalength;
io_info.ldTgtId = device_id;
-   io_info.r1_alt_dev_handle = MR_PD_INVALID;
+   io_info.r1_alt_dev_handle = MR_DEVHANDLE_INVALID;
scsi_buff_len = scsi_bufflen(scp);
io_request->DataLength = cpu_to_le32(scsi_buff_len);
 
@@ -2025,7 +2027,7 @@ megasas_build_ldio_fusion(struct megasas_instance 
*instance,
io_info.isRead && io_info.ra_capable)
fp_possible = false;
 
-   if (io_info.r1_alt_dev_handle != MR_PD_INVALID) {
+   if (io_info.r1_alt_dev_handle != MR_DEVHANDLE_INVALID) {
mrdev_priv = scp->device->hostdata;
 
if (atomic_inc_return(>fw_outstanding) >
@@ -2090,9 +2092,10 @@ megasas_build_ldio_fusion(struct megasas_instance 
*instance,
} else
scp->SCp.Status &= ~MEGASAS_LOAD_BALANCE_FLAG;
 
-   cmd->is_raid_1_fp_write = io_info.is_raid_1_fp_write;
-   if (io_info.is_raid_1_fp_write)
+   if (instance->is_ventura)
cmd->r1_alt_dev_handle = io_info.r1_alt_dev_handle;
+   else
+   cmd->r1_alt_dev_handle =