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. Ben. -- Ben Hutchings Software Developer, Codethink Ltd.
