From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753052AbdBCKbz (ORCPT ); Fri, 3 Feb 2017 05:31:55 -0500 Received: from mail-eopbgr30042.outbound.protection.outlook.com ([40.107.3.42]:7440 "EHLO EUR03-AM5-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752961AbdBCKbx (ORCPT ); Fri, 3 Feb 2017 05:31:53 -0500 From: Laurentiu Tudor To: Stuart Yoder , "gregkh@linuxfoundation.org" CC: "devel@driverdev.osuosl.org" , "linux-kernel@vger.kernel.org" , "agraf@suse.de" , "arnd@arndb.de" , "Ioana Ciornei" , Ruxandra Ioana Radulescu , Bharat Bhushan , Catalin Horghidan , Leo Li , Roy Pledge Subject: Re: [PATCH 4/9] staging: fsl-mc: don't use devres api for refcounted objects Thread-Topic: [PATCH 4/9] staging: fsl-mc: don't use devres api for refcounted objects Thread-Index: AQHSfIBxYGFGb96MV0CtzAk3mrXO0KFWaLyAgACvtoA= Date: Fri, 3 Feb 2017 10:31:50 +0000 Message-ID: <58945C15.9050002@nxp.com> References: <20170201114329.21276-1-laurentiu.tudor@nxp.com> <20170201114329.21276-5-laurentiu.tudor@nxp.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=laurentiu.tudor@nxp.com; x-ms-exchange-messagesentrepresentingtype: 1 x-originating-ip: [192.88.146.1] x-microsoft-exchange-diagnostics: 1;VI1PR0401MB1725;7:3+ktFd0mNPHDZiwKrRVnEBgQg9nZ76fvVqqQHcLKLuM/lyhDSVK0e0kiIYkYy9hZM19OpMrjkT0LCMtejJDOcAHmbJWIblUEi09IilUL+koGzwAaSqdG6XFZtrdTwim4S7F5cOi3th365+S08Z2vtTkjI6jgmXiqDtxCamP6dYW7c53Uh9fwnh4DDu1RHhoRRHLcJVOmBchqKHD/L6YngOCYl0zGvoHkNx5COidfcwHOrXUnGI1uK5RbjmSAxfjoDe4VmV9Al3Yo/gxhi3AMddKQPNQu2RXePfqS5sZ8etXZzSgh575EfWgbfXS+KhqzyDLZ9VidEqUsA90cJGomHT1HNuhG6DOLiF/hbPRc8iM3RP1slwIUoaP0gES+8t6yciz0v9Q/ycl5gCTbAuYQtqybTLfngb532dd/EEd2kWvuRKUrsxR5X1xf1D2m1wbhkKPdhN44FvTMJthF4yGlbPWnLBfCUFfX5SzDASsUzHOi/SdR743G97cqyjef4OJG851pVD0Sa5/55BzXIzq/sw== x-forefront-antispam-report: SFV:SKI;SCL:-1SFV:NSPM;SFS:(10009020)(6009001)(7916002)(39450400003)(39850400002)(39860400002)(39410400002)(39840400002)(24454002)(377454003)(13464003)(189002)(199003)(3660700001)(59896002)(53546003)(25786008)(99286003)(2906002)(305945005)(4326007)(7736002)(3280700002)(92566002)(6486002)(6246003)(6506006)(38730400001)(6436002)(77096006)(54906002)(36756003)(86362001)(6512007)(66066001)(76176999)(5660300001)(65816999)(189998001)(6116002)(102836003)(68736007)(229853002)(2950100002)(99136001)(8936002)(106116001)(87266999)(54356999)(50986999)(8676002)(122556002)(81166006)(106356001)(53936002)(81156014)(2900100001)(2501003)(105586002)(5001770100001)(97736004)(3846002)(101416001)(33656002)(80316001);DIR:OUT;SFP:1101;SCL:1;SRVR:VI1PR0401MB1725;H:VI1PR0401MB1856.eurprd04.prod.outlook.com;FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; x-ms-office365-filtering-correlation-id: 5179b400-3270-459e-489a-08d44c1fdd2d x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001)(48565401081);SRVR:VI1PR0401MB1725; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(9452136761055)(185117386973197); x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(6040375)(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001)(6055026)(6041248)(20161123555025)(20161123560025)(20161123564025)(20161123562025)(20161123558025)(6072148);SRVR:VI1PR0401MB1725;BCL:0;PCL:0;RULEID:;SRVR:VI1PR0401MB1725; x-forefront-prvs: 02070414A1 spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="Windows-1252" Content-ID: <57BC951A88691D4BB9CCA9DBFA99E3B3@eurprd04.prod.outlook.com> MIME-Version: 1.0 X-OriginatorOrg: nxp.com X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Feb 2017 10:31:50.1559 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR0401MB1725 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id v13AVxDo021599 On 02/03/2017 02:02 AM, Stuart Yoder wrote: > > >> -----Original Message----- >> From: laurentiu.tudor@nxp.com [mailto:laurentiu.tudor@nxp.com] >> Sent: Wednesday, February 01, 2017 5:43 AM >> To: gregkh@linuxfoundation.org >> Cc: devel@driverdev.osuosl.org; linux-kernel@vger.kernel.org; agraf@suse.de; arnd@arndb.de; Ioana >> Ciornei ; Ruxandra Ioana Radulescu ; Bharat Bhushan >> ; Stuart Yoder ; Catalin Horghidan >> ; Leo Li ; Roy Pledge ; Laurentiu >> Tudor >> Subject: [PATCH 4/9] staging: fsl-mc: don't use devres api for refcounted objects >> >> From: Laurentiu Tudor >> >> Mixing two memory management systems, in this case >> managed device resource api and refcounted objects >> is a bad idea. Lifetime of an object is controlled >> by its refcount so allocating it with other apis >> that have their own lifetime control is not ok. >> Drop devm_*() apis in favor of plain allocations. >> >> While at it, let's drop the slab cache for objects >> until we actually have proof that it improves >> performance. This allows for some code cleanup. > > Those 2 changes (dropping devm_* apis and slab cache > changes) are 2 orthogonal things, right? It would > be better to split those into separate patches. Mixing > them makes it harder to review. > >> Signed-off-by: Laurentiu Tudor >> --- >> drivers/staging/fsl-mc/bus/fsl-mc-bus.c | 43 +++++---------------------------- >> 1 file changed, 6 insertions(+), 37 deletions(-) >> >> diff --git a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c >> index 6601bde..c493427 100644 >> --- a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c >> +++ b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c >> @@ -27,8 +27,6 @@ >> #include "fsl-mc-private.h" >> #include "dprc-cmd.h" >> >> -static struct kmem_cache *mc_dev_cache; >> - >> /** >> * Default DMA mask for devices on a fsl-mc bus >> */ >> @@ -422,17 +420,12 @@ bool fsl_mc_is_root_dprc(struct device *dev) >> static void fsl_mc_device_release(struct device *dev) >> { >> struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev); >> - struct fsl_mc_bus *mc_bus = NULL; >> >> kfree(mc_dev->regions); >> - >> - if (strcmp(mc_dev->obj_desc.type, "dprc") == 0) >> - mc_bus = to_fsl_mc_bus(mc_dev); >> - >> - if (mc_bus) >> - devm_kfree(mc_dev->dev.parent, mc_bus); >> + if (!strcmp(mc_dev->obj_desc.type, "dprc")) >> + kfree(to_fsl_mc_bus(mc_dev)); >> else >> - kmem_cache_free(mc_dev_cache, mc_dev); >> + kfree(mc_dev); >> } >> >> /** >> @@ -457,7 +450,7 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc, >> /* >> * Allocate an MC bus device object: >> */ >> - mc_bus = devm_kzalloc(parent_dev, sizeof(*mc_bus), GFP_KERNEL); >> + mc_bus = kzalloc(sizeof(*mc_bus), GFP_KERNEL); >> if (!mc_bus) >> return -ENOMEM; >> >> @@ -466,7 +459,7 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc, >> /* >> * Allocate a regular fsl_mc_device object: >> */ >> - mc_dev = kmem_cache_zalloc(mc_dev_cache, GFP_KERNEL); >> + mc_dev = kzalloc(sizeof(*mc_dev), GFP_KERNEL); >> if (!mc_dev) >> return -ENOMEM; >> } >> @@ -561,10 +554,7 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc, >> >> error_cleanup_dev: >> kfree(mc_dev->regions); >> - if (mc_bus) >> - devm_kfree(parent_dev, mc_bus); >> - else >> - kmem_cache_free(mc_dev_cache, mc_dev); >> + kfree(mc_bus ? (void *)mc_bus : (void *)mc_dev); >> >> return error; >> } >> @@ -578,23 +568,11 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc, >> */ >> void fsl_mc_device_remove(struct fsl_mc_device *mc_dev) >> { >> - struct fsl_mc_bus *mc_bus = NULL; >> - >> - kfree(mc_dev->regions); >> - >> /* >> * The device-specific remove callback will get invoked by device_del() >> */ >> device_del(&mc_dev->dev); >> put_device(&mc_dev->dev); >> - >> - if (strcmp(mc_dev->obj_desc.type, "dprc") == 0) >> - mc_bus = to_fsl_mc_bus(mc_dev); >> - >> - if (mc_bus) >> - devm_kfree(mc_dev->dev.parent, mc_bus); >> - else >> - kmem_cache_free(mc_dev_cache, mc_dev); >> } > > The above changes in fsl_mc_device_remove(), I think > actually belong in patch #3 of this series. > Agree. I think I end up doing a double free here. --- Thanks & Best Regards, Laurentiu