Re: [PATCH v2 03/39] megaraid_sas: raid 1 fast path code optimize
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
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
> > +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
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
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 SSigned-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 =