Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753700AbbGFRis (ORCPT ); Mon, 6 Jul 2015 13:38:48 -0400 Received: from comal.ext.ti.com ([198.47.26.152]:44593 "EHLO comal.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751388AbbGFRip (ORCPT ); Mon, 6 Jul 2015 13:38:45 -0400 Date: Mon, 6 Jul 2015 12:38:29 -0500 From: Felipe Balbi To: Alan Stern CC: Ruslan Bilovol , "Balbi, Felipe" , "linux-usb@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Krzysztof Opasiak , Peter Chen , "gregkh@linuxfoundation.org" , Andrzej Pietrasiewicz , Maxime Ripard Subject: Re: [PATCH v5 5/5] usb: gadget: udc-core: independent registration of gadgets and gadget drivers Message-ID: <20150706173829.GA20779@saruman.tx.rr.com> Reply-To: References: MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="/04w6evG8XlLl3ft" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3523 Lines: 83 --/04w6evG8XlLl3ft Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Jun 27, 2015 at 07:47:28PM -0400, Alan Stern wrote: > On Sun, 28 Jun 2015, Ruslan Bilovol wrote: >=20 > > > Weren't you going to replace this loop with a simple list_del()? IIR= C, > > > this is the third time I have asked you to make this change. > >=20 > > I understand the improvement that replacing this loop with a list_del() > > may bring for us, but I disagree with doing it in this particular case. > >=20 > > The reason is simple. The usb_gadget_unregister_driver() funciton is > > externally visible so we can get junk as input. Current implementation > > checks passed pointer and only after that does list_del(), or > > returns -EINVAL. Your variant will do list_del() unconditionally, that > > may cause a kernel crash or unexpected behavior in case of junk > > passed with *driver. The list_del_init() usage can't help here since > > there is no way to check that list_head structure is initialized with c= orrect > > data or contains junk. >=20 > That's right. >=20 > > There is no noticeable performance loss with current implementation,just > > because current use case is pretty simple: one gadget driver per one UD= C, > > and usually there is only one UDC per machine (or rare cases with few > > UDCs), thus number of pending gadget drivers is relatively small. > > We can return back to this discussion if someone needs to register > > many gadget drivers, and want to improve performance, because > > there are few existing places (not created by me) in this file that uses > > same approach of walking through list of registered gadget drivers. > >=20 > > As a bottom line, choosing between stability and little performance > > improvement, I prefer stability. >=20 > It's not really a question of code size or performance. As you say, > the difference in each is minimal. >=20 > It _is_ a question of style. Adding unnecessary code to check for > something that shouldn't need to be checked looks bad. Other kernel > developers reading that code will notice it and wonder why it's there. = =20 > That's the argument for getting rid of the loop. yeah, if someone gives us junk as input, they deserve to oops. list_del() is enough. --=20 balbi --/04w6evG8XlLl3ft Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJVmr0UAAoJEIaOsuA1yqRECmoP/27pE0XuKbJn9pcnr1mtb7hV QqjI/qPWZZcFosSMHKc37yp1RVPjVdOOswOFbv0uDODumhBsCjHnHt4k1bG8+Dki ed4uE/Zj5tSL07W6xjAtgbERSwkrpxiMgGjKp06qs9RLKFh7TNXW3u6YVYqCgsk1 rzCc9P2hnTe70zqBehkCFJap5+Zbo7IAcD6WAWU98KAyD0tJKcJY1CSkLiHvDnNH HOp1cIkOkLYtkCqp8+YETTg4Aqb3TRlbqH9YrUHKOLty9P4bVt3lu+iYyuX3neCP MQ7tH3jkhvTUNX2rScr9j2SsrBUe6mZKFnSpgGGJ5ioxtXvG4otaGZkQgoE5JAVw GjIqBcT/HWULugZuTmh4tXC3VWIu+X+7yGW/+SlnDmzzxk3ih8fJzRtmirhYLJa+ 3ISOWEaqF0Z8pxZ90DJul98vKZ04FNy98YBBNUzgBU+ZnvbppRXtp8qi3ODkQbDr dOxY9K1hQ8OYBZX/cKe8uCEg7s0yZbl3XmEAMw8xD2cG77dGTkQJGtEZkWDfCxhA 4pzN10nWD76YJzpY2v9EtaKbSCeLGCgyZ2u7Akvb9uXlTYl4RHj9Y4p4d0IlsLBo 6h+NYY+FoQWxW8S9GVCpcZzKTInPNOxR3COLPYG5ZIL5yRWtCqTdD1522DffNbOc PMLuZnCyh3GmgQ4lUyjd =RPVl -----END PGP SIGNATURE----- --/04w6evG8XlLl3ft-- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/