Re: [PATCH 2/2] drivers/usb: Delete XHCI command timer if necessary

[Greg KH <gregkh@linuxfoundation.org>]

On Wed, Aug 12, 2015 at 01:55:34PM +0300, Mathias Nyman wrote:
> On 11.08.2015 11:15, Oliver Neukum wrote:
> > On Mon, 2015-08-03 at 16:07 +0300, Mathias Nyman wrote:
> >> From: Gavin Shan <gwshan@linux.vnet.ibm.com>
> >>
> >> When xhci_mem_cleanup() is called, it's possible that the command
> >> timer isn't initialized and scheduled. For those cases, to delete
> >> the command timer causes soft-lockup as below stack dump shows.
> >>
> >> The patch avoids deleting the command timer if it's not scheduled
> >> with the help of timer_pending().
> > 
> > Are you sure this is safe? timer_pending() will not show you that
> > the timer function is running. It looks like you introduced a race
> > between timeout and cleanup.
> > 
> 
> Looking at it in more detail you're right.
> 
> Fortunately this can only happen in cases where xhci is already hosed
> (no command response for 5 seconds), and we are at the same time
> anyway about to remove xhci.
> 
> Doesn't this mean that all cases with
> if (timer_pending(&timer))
> 	del_timer_sync(&timer)
> 
> is just basically the same as a plain del_timer(&timer)?
> 
> Anyways, turns out that the error path in xhci initialization code can end up calling
> del_timer_sync() before timer is initialized. This should be fixed by re-arranging
> some code in xhci initialization instead.
> 
> Greg, should this be reverted in rc7?
> I think that the possible side effect of this patch is still lesser the original
> issue.     

Just fix it "right" in a new patch.

thanks,

greg k-h