From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com ([134.134.136.65]:38101 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751990AbcKXJEO (ORCPT ); Thu, 24 Nov 2016 04:04:14 -0500 From: Felipe Balbi To: Mathias Nyman , linux@roeck-us.net Cc: linux-usb@vger.kernel.org, Mathias Nyman , stable@vger.kernel.org Subject: Re: [RFT PATCH 1/1] xhci: free xhci virtual devices with leaf nodes first In-Reply-To: <1479903867-561-1-git-send-email-mathias.nyman@linux.intel.com> References: <582DC88C.5040308@linux.intel.com> <1479903867-561-1-git-send-email-mathias.nyman@linux.intel.com> Date: Thu, 24 Nov 2016 11:02:29 +0200 Message-ID: <8760ndqmhm.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Sender: stable-owner@vger.kernel.org List-ID: --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, Mathias Nyman writes: > the tt_info provided by a HS hub might be in use to by a child device > Make sure we free the devices in the correct order. > > This is needed in special cases such as when xhci controller is > reset when resuming from hibernate, and all virt_devices are freed. > > Also free the virt_devices starting from max slot_id as children > more commonly have higher slot_id than parent. > > CC: > Signed-off-by: Mathias Nyman > > --- > > Guenter Roeck, does this work for you? > > A rework of how tt_info is stored and used might be needed, > but that will take some time and won't go to stable as easily. > --- > drivers/usb/host/xhci-mem.c | 38 ++++++++++++++++++++++++++++++++++++-- > 1 file changed, 36 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c > index 6afe323..b3a5cd8 100644 > --- a/drivers/usb/host/xhci-mem.c > +++ b/drivers/usb/host/xhci-mem.c > @@ -979,6 +979,40 @@ void xhci_free_virt_device(struct xhci_hcd *xhci, in= t slot_id) > xhci->devs[slot_id] =3D NULL; > } >=20=20 > +/* > + * Free a virt_device structure. > + * If the virt_device added a tt_info (a hub) and has children pointing = to > + * that tt_info, then free the child first. Recursive. > + * We can't rely on udev at this point to find child-parent relationship= s. > + */ > +void xhci_free_virt_devices_depth_first(struct xhci_hcd *xhci, int slot_= id) > +{ > + struct xhci_virt_device *vdev; > + struct list_head *tt_list_head; > + struct xhci_tt_bw_info *tt_info, *next; > + int i; > + > + vdev =3D xhci->devs[slot_id]; > + if (!vdev) > + return; > + > + tt_list_head =3D &(xhci->rh_bw[vdev->real_port - 1].tts); > + list_for_each_entry_safe(tt_info, next, tt_list_head, tt_list) { > + /* is this a hub device that added a tt_info to the tts list */ > + if (tt_info->slot_id =3D=3D slot_id) { if (tt_info->slot_id !=3D slot_id) continue; > + /* are any devices using this tt_info? */ > + for (i =3D 1; i < HCS_MAX_SLOTS(xhci->hcs_params1); i++) { off-by-one here ? Why is i starting from 1? > + vdev =3D xhci->devs[i]; > + if (vdev && (vdev->tt_info =3D=3D tt_info)) if (!vdev || vdev->tt_info !=3D tt_info) continue; > + xhci_free_virt_devices_depth_first( > + xhci, i); > + } > + } > + } > + /* we are now at a leaf device */ > + xhci_free_virt_device(xhci, slot_id); > +} > + > int xhci_alloc_virt_device(struct xhci_hcd *xhci, int slot_id, > struct usb_device *udev, gfp_t flags) > { > @@ -1829,8 +1863,8 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci) > } > } >=20=20 > - for (i =3D 1; i < MAX_HC_SLOTS; ++i) > - xhci_free_virt_device(xhci, i); > + for (i =3D HCS_MAX_SLOTS(xhci->hcs_params1); i > 0; i--) converting MAX_HC_SLOTS to HCS_MAX_SLOTS(xhci->hcs_params1) seems unrelated to $subject. Perhaps just mention on commit log? =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEElLzh7wn96CXwjh2IzL64meEamQYFAlg2rKUACgkQzL64meEa mQYclA/9HWyEG4EkG2ZD8WbV2xw0faueDKZqFJWdu54RIsmdzn1ItV3/Q/viaW/Q h5gHbX0yXtsAZgjLoxhQ0GyVgF/zXz4RJlqId/bHzCSZhQCxbIyHZ7yNmfhFUsIX LIAP+YymqSsml0vUMotvcZR/djAV5D0i7s5L3d9O+OWJBLRYdKIJ6ERTjyJYIjqf elI7Nki+r5NYsF+LhrIvQ4b2WYdfCYiFCOuHfJW5ZItnjvLeDtv4uElKMmYr4M2O yZWE/AMTOz7i9cbV9Z3fBb7VL2RjygPfQ4K0iRoXjOZdNXMPuwjlGj7cDZXsgAds eHb9QIQXKhyGGTwlKIPXi2S+yZkMIewbzh+G5Q2P4xdUdxVwWMkt5I46SmcQMkWO psbvAm4rXSI8Ch6y/gfOJrE1hhaujJR4k075kUllhDj2UuVg9Q9BmJUjgz8rrjwa 0HAA5klI5TlbvNMurIZlr451ll9v7eu2hvTzLmiYbvWrB25ynBZDWEuFDAZCC+ba ycj/FuRezu8ofGXae0kFm84ZbNdsZvcXX2/j28NTi4Itm58KnrsU3oZT/pOMDc27 Pxa9C369HUcjvnkB+z9+5lODcKncus3dUIm6dGcs4Nj2Ak7OadLdPfT/I3X37pNA DvB7D63lKuvVUpRPPWI2ior2XM7MEI/2OtyL7xY4lHUPQlF7u6A= =t7v5 -----END PGP SIGNATURE----- --=-=-=--