On Wed, 2018-03-21 at 17:55 +0000, Madhani, Himanshu wrote: > Hi Ben, > > > On Mar 21, 2018, at 10:45 AM, Ben Hutchings <ben.hutchings@xxxxxxxxxxxxxxx> wrote: > > > > On Wed, 2018-03-21 at 17:17 +0000, Madhani, Himanshu wrote: > > > Hi Ben, > > > > > > > On Mar 20, 2018, at 2:36 PM, Ben Hutchings <ben.hutchings@xxxxxxxxxxxxxxx> wrote: > > > > > > > > qla2x00_init_timer() calls add_timer() on the iocb timeout timer, > > > > which means the timeout function pointer and any data that the > > > > function depends on must be initialised beforehand. > > > > > > > > Move this initialisation before each call to qla2x00_init_timer(). In > > > > some cases qla2x00_init_timer() initialises a completion structure > > > > needed by the timeout function, so move the call to add_timer() after > > > > that. > > > > [...] > > > What motivated this patch? are you debugging any crash which was > > > helped by moving code around? > > > > I saw the recent fix that added a call to del_timer(), noticed the lack > > of synchronisation and then kept looking further. > > > > > What I see from this patch is that its moving iocb.cmd.timeout field > > > before qla2x00_init_timer(), > > > which are completely different from each other. > > > > How are they "completely different"? qla2x00_init_timer() starts a > > timer that will call qla2x00_sp_timeout(), which in turn uses the > > iocb_cmd.timeout function pointer. We should not assume that any > > initialisation code after the call to add_timer() will run before the > > timer expires, since the scheduler might run other tasks for the whole > > time. It's unlikely but not impossible. > > > > I see. I used “completely different” due to the lack of better wording. > > I wanted to get context and understand if there was any issue that would > have helped with these changes. > > your explanation does help and I agree that there is window where timer() will > pop before timeout is initialized. > > Let me put this patch in our test cycle for couple days and will respond on this one Thanks. Ben. -- Ben Hutchings Software Developer, Codethink Ltd.
