On Wed, Aug 8, 2018 at 1:27 AM, Bart Van Assche <Bart.VanAssche@xxxxxxx> wrote:
> On Tue, 2018-08-07 at 21:46 +0530, Sreekanth Reddy wrote:
>> [Sreekanth] In the patch description I mentioned that I have done a
>> manual mistake and I am correcting with this patch.
>>
>> Hope I have answered all of your quires.
>
> Not yet unfortunately. Why do you consider the current implementation a
> wrong?
In function mpt3sas_base_clear_st(),
st->cb_idx = 0xFF;
st->direct_io = 0;
st->smid = 0;
atomic_set(&ioc->chain_lookup[st->smid - 1].chain_offset, 0);
In the above code we can see that smid is reset to zero before
resetting the chain_offset to zero in chain_lookup[] table for
corresponding IO request entry (i.e. smid -1 th entry). So in the
above code driver is logically resetting the chain_offset to zero for
the -1th entry of the chain_lookup[] table for all the IO requests,
which I am say it wrong. I am saying that above code should be like
below,
st->cb_idx = 0xFF;
st->direct_io = 0;
atomic_set(&ioc->chain_lookup[st->smid - 1].chain_offset, 0);
st->smid = 0;
I.e. reset the smid variable to zero only after resetting the
chain_offset for the corresponding IO request (i.e. for smid -1 th
entry) of the chain_lookup[] table.
> Why is the patch at the start of this e-mail thread considered a
> fix? Which behavior changes due to this patch? If this patch is a bug fix,
> how can the bug be triggered and why is this patch considered the right way
> to fix that bug?
See while posting the patch "mpt3sas: Fix calltrace observed while
running IO & reset" in hurry I made a manual mistake that I added
"st->smid = 0" line before "atomic_set(&ioc->chain_lookup[st->smid -
1].chain_offset, 0);" line in mpt3sas_base_clear_st() function instead
I should have added "st->smid = 0" line after the atomic set line.
Which I am correcting my mistake with this patch.
Thanks,
Sreekanth
>
> Thanks,
>
> Bart.
>
