Re: [PATCH] megaraid_sas : Modify return value of megasas_issue_blocked_cmd() and wait_and_poll() to consider command status returned by firmware

[Hannes Reinecke <hare@suse.de>]

On 05/06/2015 03:31 PM, Sumit.Saxena@avagotech.com wrote:
> This patch is rebased on top of recently sent 18 patches(submitted by me) for megaraid_sas driver.
> 
> Change the return value of wait_and_poll() and megsas_issue_blocked_cmd() based on MFI_STAT returned by firmware for that command. Earlier driver always
> send return type based on command completion (but never check MFI_STAT_OK for that command), so even if command is failed by firmware still driver will
> return SUCCESS status from these functions wait_and_poll() and megsas_issue_blocked_cmd() and if caller of these functions does not check command status
> (MFI_STAT), then it may endup using invalid data returned in DMA buffers(one of the example is megasas_ld_list_query DCMD). Best thing to avoid this type
> of issue is do error handling and set proper return type from caller function wait_and_poll() and megsas_issue_blocked_cmd().
> 
> The change proposed in this patch will fix the regression introduced in patch- "90dc9d9 megaraid_sas : MFI MPT linked list corruption fix" inside function
> megasas_ld_list_query().
> Prior to this MFI MPT linked list corruption fix patch, megasas_ld_list_query() function used to check DCMD status(returned by firmware) but with
> this linked list corruption fix patch, DCMD status will not be checked inside function megasas_ld_list_query() and introduced this issue of wrong data
> being used by function megasas_ld_list_query().
> 
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Kashyap Desai <kashyap.desai@avagotech.com>
> Signed-off-by: Sumit Saxena <sumit.saxena@avagotech.com>
> ---
>  drivers/scsi/megaraid/megaraid_sas.h        |    2 +-
>  drivers/scsi/megaraid/megaraid_sas_base.c   |   67 +++++++++++----------------
>  drivers/scsi/megaraid/megaraid_sas_fusion.c |    3 +-
>  3 files changed, 30 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/scsi/megaraid/megaraid_sas.h b/drivers/scsi/megaraid/megaraid_sas.h
> index 53a3c3f..20c3754 100644
> --- a/drivers/scsi/megaraid/megaraid_sas.h
> +++ b/drivers/scsi/megaraid/megaraid_sas.h
> @@ -1894,7 +1894,7 @@ struct megasas_cmd {
>  
>  	u32 index;
>  	u8 sync_cmd;
> -	u8 cmd_status;
> +	u8 cmd_status_drv;
>  	u8 abort_aen;
>  	u8 retry_for_fw_reset;
>  
Can you please avoid the renaming here?
It doesn't serve any purpose, and keeping it will make the diff smaller.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html