Re: [PATCH][SCSI] megaraid_sas: Fix synchronization problem between sysPD IO path and AEN path

[Tomas Henzl <thenzl@redhat.com>]

On 10/16/2013 01:34 PM, Sumit.Saxena@lsi.com wrote:
> There is syncronization problem between sysPD IO path and AEN path. Driver maintains instance->pd_list[] array, which will get updated(by calling function megasas_get_pd_list[]), whenever any of below events occurs-

Hi Sumit,
- I'm a bit confused here- there are two threads which might access the same array, but the problem is still there
  when the second thread accesses the array during the final memcpy, I have expected that you will add some locking,
  but maybe I'm missing something.
- now the code zeroes the pd_list even when the 
  (ret == 0 && (ci->count < (MEGASAS_MAX_PD_CHANNELS * MEGASAS_MAX_DEV_PER_CHANNEL)))
  is not true. This is I think new - is that intentional?
Thanks, Tomas
 

>
>
>
>
>
>
> MR_EVT_PD_INSERTED
> MR_EVT_PD_REMOVED
> MR_EVT_CTRL_HOST_BUS_SCAN_REQUESTED
> MR_EVT_FOREIGN_CFG_IMPORTED
>
> At same time running sysPD IO will be accessing the same array instance->pd_list[], which is getting updated in AEN path, because
> of this IO may not get correct PD info from instance->pd_list[] array.
>
> Signed-off-by: Adam Radford <adam.radford@lsi.com>
> Signed-off-by: Sumit Saxena <sumit.saxena@lsi.com>
> ---
> diff --git a/drivers/scsi/megaraid/megaraid_sas.h b/drivers/scsi/megaraid/megaraid_sas.h
> index 0c73ba4..e9e543c 100644
> --- a/drivers/scsi/megaraid/megaraid_sas.h
> +++ b/drivers/scsi/megaraid/megaraid_sas.h
> @@ -1531,6 +1531,7 @@ struct megasas_instance {
>  	struct megasas_register_set __iomem *reg_set;
>  	u32 *reply_post_host_index_addr[MR_MAX_MSIX_REG_ARRAY];
>  	struct megasas_pd_list          pd_list[MEGASAS_MAX_PD];
> +	struct megasas_pd_list          local_pd_list[MEGASAS_MAX_PD];
>  	u8     ld_ids[MEGASAS_MAX_LD_IDS];
>  	s8 init_id;
>  
> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
> index e62ff02..83ebc75 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> @@ -3194,21 +3194,23 @@ megasas_get_pd_list(struct megasas_instance *instance)
>  	     (le32_to_cpu(ci->count) <
>  		  (MEGASAS_MAX_PD_CHANNELS * MEGASAS_MAX_DEV_PER_CHANNEL))) {
>  
> -		memset(instance->pd_list, 0,
> +		memset(instance->local_pd_list, 0,
>  			MEGASAS_MAX_PD * sizeof(struct megasas_pd_list));
>  
>  		for (pd_index = 0; pd_index < le32_to_cpu(ci->count); pd_index++) {
>  
> -			instance->pd_list[le16_to_cpu(pd_addr->deviceId)].tid	=
> +			instance->local_pd_list[le16_to_cpu(pd_addr->deviceId)].tid	=
>  				le16_to_cpu(pd_addr->deviceId);
> -			instance->pd_list[le16_to_cpu(pd_addr->deviceId)].driveType	=
> +			instance->local_pd_list[le16_to_cpu(pd_addr->deviceId)].driveType	=
>  							pd_addr->scsiDevType;
> -			instance->pd_list[le16_to_cpu(pd_addr->deviceId)].driveState	=
> +			instance->local_pd_list[le16_to_cpu(pd_addr->deviceId)].driveState	=
>  							MR_PD_STATE_SYSTEM;
>  			pd_addr++;
>  		}
>  	}
>  
> +	memcpy(instance->pd_list, instance->local_pd_list,
> +		sizeof(instance->pd_list));
>  	pci_free_consistent(instance->pdev,
>  				MEGASAS_MAX_PD * sizeof(struct MR_PD_LIST),
>  				ci, ci_h);
>
> --
> 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