Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935077AbdCJKzx (ORCPT ); Fri, 10 Mar 2017 05:55:53 -0500 Received: from mga05.intel.com ([192.55.52.43]:54593 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934657AbdCJKzu (ORCPT ); Fri, 10 Mar 2017 05:55:50 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,140,1486454400"; d="asc'?scan'208";a="58555085" From: Felipe Balbi To: Michal Nazarewicz Cc: linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org, Michal Nazarewicz Subject: Re: [PATCH] usb: gadget: f_fs: simplify ffs_dev name handling In-Reply-To: <20170301143005.23396-1-mina86@mina86.com> References: <20170301143005.23396-1-mina86@mina86.com> Date: Fri, 10 Mar 2017 12:54:41 +0200 Message-ID: <8737elz9ji.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3605 Lines: 95 --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hi, Michal Nazarewicz writes: > Currently ffs_dev::name can be either allocated by the client of > the ffs_dev structure or by the f_fs.c core itself. The former > is used by g_ffs while the latter happens with configfs. > > Historically, g_ffs did not need to allocate separate buffer for > the name so what is now f_fs.c core never cared about freeing > that space. With configfs the name needs to be copied since the > memory is not guaranteed to be availeble after ffs_set_inst_name > finishes. > > The complication is therefore here to avoid allocations in the > g_ffs case but it complicates the code inproportinally to > benefits it provides. In particular, g_ffs is considered > =E2=80=98legacy=E2=80=99 so optimising for its sake is unlikely to be wor= th the > effort. > > With that observation in mind, simplify the code by unifying the > code paths in g_ffs and configfs paths. Furthermore, instead of > allocating a new buffer for the name, simply embed it in the > ffs_dev structure. This further makes the memory management > less convoluted and error-prone. > > The configfs interface for functionfs imposed a limit of 40 > characters for the name so this results in a 41-byte buffer > added to the structure. (For short names this may lead to > wasted memory but the actual amount is not immediately obvious > and depends on pointer size and which slab buckets the structure > and name would fall into). > > Signed-off-by: Michal Nazarewicz > --- > drivers/usb/gadget/function/f_fs.c | 79 ++++++++------------------------= ------ > drivers/usb/gadget/function/u_fs.h | 11 +++--- > 2 files changed, 22 insertions(+), 68 deletions(-) > > diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/func= tion/f_fs.c > index f5d6bf527aac..6ed4da6e4474 100644 > --- a/drivers/usb/gadget/function/f_fs.c > +++ b/drivers/usb/gadget/function/f_fs.c > @@ -245,7 +245,6 @@ EXPORT_SYMBOL_GPL(ffs_lock); >=20=20 > static struct ffs_dev *_ffs_find_dev(const char *name); > static struct ffs_dev *_ffs_alloc_dev(void); > -static int _ffs_name_dev(struct ffs_dev *dev, const char *name); > static void _ffs_free_dev(struct ffs_dev *dev); > static void *ffs_acquire_dev(const char *dev_name); > static void ffs_release_dev(struct ffs_data *ffs_data); > @@ -3277,13 +3276,13 @@ static LIST_HEAD(ffs_devices); >=20=20 > static struct ffs_dev *_ffs_do_find_dev(const char *name) > { > - struct ffs_dev *dev; > + if (name) { how about avoiding the extra indentation level? if (!name) return NULL; [...] =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEElLzh7wn96CXwjh2IzL64meEamQYFAljChfEACgkQzL64meEa mQYBxRAAhiDuPZ8vBKxkFWqYd8Da04W2IpZ6r4Nz9dgvQrJR6bLQxNrK6XkG+UZT FSJfwCdIfSkyPuMBvMsLZAQg8YMLatWWxFptwQTUPPw5ka4pI3xeX5MPzCO+mIBP x/lyU/PysbH3N4lJJf/aA1JWfXcaGa99jbbWWe3y+yHme0Ui3FXBbgIxTYkeYS6Y RB8iptV8uyITDz+1C5f1O0Md2qkUxMx4Qk64BQXapVeVJnMBvj2eDSuDLi8ls+A5 NBuEAzws1P42h4sx5+B0Tac3og6Kc3neJ+rd4ZAzpEt+87xDrlGxkAMtbNusH6LE 2AFRD2qHlllU0pQoUNFPH/ZlhoOCkXp6Tg2qMxcUJHL7Y4VeBsP18a70TAeb5mmH SDPPehcvPQOk2Vc8rJbyB4XR84YUWAs+pe7b5SJZRuDZ8X91eLzPW5cBmiNdJdbo lRbblYU71foZBP3vuwdvVx7JN6dzLjZW4QCMmzlGBIMp5hi3enlX/0RQpz5TFwe4 lT4zKSEK56whYLI4uweymajjZpSlStaEtGROk3Vwa242PCrL48o4bQcurwHj0lUn DQZYr/gN3WSbUEhOVyj8IseUneka93pGpp8wlv9pnJL3gNOJfGRFN29MUIBSSGNU WmObTWDcw9rQLKevslLamM4JjbAzTj32+PEvUo5MZ91PKATZ824= =wiNF -----END PGP SIGNATURE----- --=-=-=--