From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.linuxfoundation.org ([140.211.169.12]:60215 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752850AbbHLQS6 (ORCPT ); Wed, 12 Aug 2015 12:18:58 -0400 Date: Wed, 12 Aug 2015 09:18:57 -0700 From: Greg KH To: Mathias Nyman Cc: Oliver Neukum , linux-usb@vger.kernel.org, Gavin Shan , stable@vger.kernel.org Subject: Re: [PATCH 2/2] drivers/usb: Delete XHCI command timer if necessary Message-ID: <20150812161857.GA29595@kroah.com> References: <1438607269-8977-1-git-send-email-mathias.nyman@linux.intel.com> <1438607269-8977-3-git-send-email-mathias.nyman@linux.intel.com> <1439280938.6524.2.camel@suse.com> <55CB2626.3060000@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55CB2626.3060000@linux.intel.com> Sender: stable-owner@vger.kernel.org List-ID: 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 > >> > >> 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