usb HC busted?

From: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
To: Mathias Nyman <mathias.nyman@linux.intel.com>
Cc: Alan Stern <stern@rowland.harvard.edu>,
	Greg KH <gregkh@linuxfoundation.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	Mathias Nyman <mathias.nyman@intel.com>,
	linux-usb@vger.kernel.org, lukaszx.szulc@intel.com,
	Christoph Hellwig <hch@lst.de>,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	iommu@lists.linux-foundation.org
Subject: usb HC busted?
Date: Sat, 21 Jul 2018 11:55:47 +0100	[thread overview]
Message-ID: <20180721105509.hjocon6ngk2liwo4@debian> (raw)

Hi Mathias,

On Fri, Jul 20, 2018 at 01:54:21PM +0100, Sudip Mukherjee wrote:
> Hi Mathias,
> 
> On Fri, Jul 20, 2018 at 02:10:58PM +0300, Mathias Nyman wrote:
> > On 19.07.2018 20:32, Sudip Mukherjee wrote:
> > > Hi Mathias,
> > > 
> > > On Thu, Jul 19, 2018 at 06:42:19PM +0300, Mathias Nyman wrote:
> > > > > > As first aid I could try to implement checks that make sure the flushed URBs
> > > > > > trb pointers really are on the current endpoint ring, and also add some warning
> > > > > > if we are we are dropping endpoints with URBs still queued.
> > > > > 
> > > > > Yes, please. I think your first-aid will be a much better option than
> > > > > the hacky patch I am using atm.
> > > > > 
> > > > 
> <snip>
> > So poison is overwritten at e5acda58 with almost its own address, (reading backwards) e5 ac da 60, twice.
> > looks like something (32bit?)is pointing to itself twice, maybe a linked list node next and prev pointer
> > being set to point to itself as last item was removed from list.
> > 
> > The cancelled_td_list is part of struct xhci_virt_ep, so that should be fine.
> > But td_list is part of struct xhci_ring, which was freed. and we removed the URBs tds from the td_list when
> > flushing the ring after ring was freed
> > 
> > I changed the patch (attached) to make sure it doesn't touch the td_list when canceling a URB after
> > ring is freed.
> > 
> > How about this one, any improvements?
> 
> Yes, it worked. :D
> 
> So, cycle-1 = no change, just to make sure I can still reproduce the error.
> cycle-2 and cycle-3 with your patch, and there was no problem,
> slub debug was also happy.
> I am starting an autotest with this patch now, and I will have almost
> 50 cycles tested by tomorrow morning.

I can confirm that your bandaid patch has worked. Total of 67 cycles
tested till now and there was no error. Its continuing to test over the
weekend.
Thank you very much for this one. :)

I guess you will start with the proper fix, that you and Alan had been
discussing, after you are fully back to work.
---
Regards
Sudip
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html