On Fri, 2018-08-03 at 06:31 -0400, Sreekanth Reddy wrote:
> In mpt3sas_base_clear_st() function smid value is reseted in
^^^^^^^
That's not an English word. Please read
https://forum.wordreference.com/threads/set-reset-vs-setted-resetted-programming.70776/
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
> b/drivers/scsi/mpt3sas/mpt3sas_base.c
> index 902610d..94f7236 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
> @@ -1702,6 +1702,11 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
> return NULL;
>
> chain_req = &ioc->chain_lookup[smid - 1].chains_per_smid[chain_offset];
> + /* Added memory barrier to make sure that correct chain tracker
> + * is retrieved before incrementing the smid pool's chain_offset
> + * value in chain lookup table.
> + */
Please verify your patch with checkpatch and use the proper kernel comment
style.
> + smp_mb__before_atomic();
The purpose of smp_mb__before_atomic() / smp_mb__after_atomic() is to implement
acquire / release semantics. I think what you need here is a regular barrier
(smp_mb()).
> @@ -3283,8 +3288,13 @@ void mpt3sas_base_clear_st(struct MPT3SAS_ADAPTER *ioc,
> return;
> st->cb_idx = 0xFF;
> st->direct_io = 0;
> - st->smid = 0;
> atomic_set(&ioc->chain_lookup[st->smid - 1].chain_offset, 0);
> + /* Added memory barrier to make sure that smid is set to zero
> + * only after resetting corresponding smid pool's chain_offset to zero
> + * in chain lookup table.
> + */
> + smp_mb__after_atomic();
> + st->smid = 0;
The two above comments also apply to these two changes.
Thanks,
Bart.
