On Tue, Aug 7, 2018 at 7:29 PM, Bart Van Assche <Bart.VanAssche@xxxxxxx> wrote: > On Tue, 2018-08-07 at 12:10 +0530, Sreekanth Reddy wrote: >> This function _base_get_chain_buffer_tracker() is called only during >> the IO submission time and mpt3sas_base_clear_st() is called during IO >> completion time and hence I don't see any race condition here. >> >> Currently this patch is very much needed, so please consider this >> patch. May be we can start new email thread to discuss on the possible >> race conditions you are observing and I can clear your quires or I can >> fix them as on we can find the real race conditions. > > Hello Sreekanth, > > How can this patch be necessary if there are no races between I/O submission > and I/O completion? Changing the order of two memory writes only makes sense > if there is a race condition involved. Since the block layer allocates a > request tag before scsih_qcmd() is called and since mpt3sas_base_free_smid() > is called before scmd->scsi_done(scmd) in _scsih_io_done(), the request tag > is freed after the smid has been freed when I/O completes in a normal way. > So there shouldn't be a race condition in that scenario. > > By the way, you have not answered my question about why development of this > patch started. Does this patch address a theoretical concern or a real bug? > In the latter case, which scenario triggers the addressed bug? If you want > this patch to go upstream you should be able to explain why you think it is > useful, and that description should be included in the patch description. Bart, The main intention of this patch to reset the smid to zero after resetting the corresponding smid entry's chain_offset to zero. While posting "mpt3sas: Fix calltrace observed while running IO & reset" patch I have done manual mistake that I reset the smid to zero before resetting the chain_offset which is wrong. Due to which driver always's reset the chain_offset of zero th entry of chain_lookup[] table which is wrong and hence I am correcting it with this patch. After that you asked me to add memory barriers and hence I have added memory barriers in subsequent versions of the patch. Thanks, Sreekanth > > Thanks, > > Bart. >
