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.hutchi...@codethink.co.uk> 
> > 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.

Reply via email to