From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id A564A1BACE for ; Wed, 19 Dec 2018 09:42:25 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 19 Dec 2018 00:42:24 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,372,1539673200"; d="scan'208";a="119612218" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by FMSMGA003.fm.intel.com with ESMTP; 19 Dec 2018 00:42:24 -0800 Received: from fmsmsx116.amr.corp.intel.com (10.18.116.20) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.408.0; Wed, 19 Dec 2018 00:42:24 -0800 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by fmsmsx116.amr.corp.intel.com (10.18.116.20) with Microsoft SMTP Server (TLS) id 14.3.408.0; Wed, 19 Dec 2018 00:42:24 -0800 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.201]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.210]) with mapi id 14.03.0415.000; Wed, 19 Dec 2018 16:42:22 +0800 From: "Pei, Andy" To: "Xu, Rosen" , "dev@dpdk.org" , "Zhang, Tianfei" CC: "Zhang, Tianfei" Thread-Topic: [PATCH v2]/driver/raw/ifpga_rawdev: fix a memory leak bug in ifpga Thread-Index: AQHUlscycyS4F9ZyQUqSiNJ+TGC/WaWFprZggAAD9VA= Date: Wed, 19 Dec 2018 08:42:21 +0000 Message-ID: <5941F446C088714A85408FA3132CFCBB6213DC@SHSMSX101.ccr.corp.intel.com> References: <1544725998-70149-1-git-send-email-andy.pei@intel.com> <1545161721-382282-1-git-send-email-andy.pei@intel.com> <0E78D399C70DA940A335608C6ED296D73A450638@SHSMSX104.ccr.corp.intel.com> In-Reply-To: <0E78D399C70DA940A335608C6ED296D73A450638@SHSMSX104.ccr.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNjlkZjJlZjgtMDQ0Ni00ZGZlLTgwZmItMTY0MWQ5NmM5MmQ1IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoidXkrc3h2dmYrV3ZZZWZQU2RTSDRXdlZINzVZOTJWYzhXNVhndlY5Yk1jQW9UU0xYb0JnT0lDMkhKTml2WUVRbiJ9 dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="iso-2022-jp" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v2]/driver/raw/ifpga_rawdev: fix a memory leak bug in ifpga X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 19 Dec 2018 08:42:26 -0000 Hi=1B$B!$=1B(B Tianfei, After checking the code, I found that opae_adapter_alloc() function is no l= ong called. So is it Ok to delete the function? To: I agree that the free is not necessary. -----Original Message----- From: Xu, Rosen=20 Sent: Wednesday, December 19, 2018 3:16 PM To: Pei, Andy ; dev@dpdk.org Cc: Zhang, Tianfei Subject: RE: [PATCH v2]/driver/raw/ifpga_rawdev: fix a memory leak bug in i= fpga Hi Andy, > -----Original Message----- > From: Pei, Andy > Sent: Wednesday, December 19, 2018 3:35 > To: dev@dpdk.org > Cc: Xu, Rosen ; Zhang, Tianfei=20 > ; Pei, Andy > Subject: [PATCH v2]/driver/raw/ifpga_rawdev: fix a memory leak bug in=20 > ifpga >=20 > When ifpga_rawdev_create() allocate memory for a new rawdev, the=20 > original code allocate redundant memory for adapter, which is a member=20 > of the rawdev. What is actually necessary is the adapter to be=20 > initialized, not memory allocated. >=20 > fixes:ef1e8ede3da5 > cc:rosen.xu@intel.com > cc:tianfei.zhang@intel.com >=20 > Signed-off-by: AndyPei > --- > drivers/raw/ifpga_rawdev/base/opae_hw_api.c | 32 > ++++++++++++++++++++++++----- > drivers/raw/ifpga_rawdev/base/opae_hw_api.h | 4 +++- > drivers/raw/ifpga_rawdev/ifpga_rawdev.c | 7 +++---- > 3 files changed, 33 insertions(+), 10 deletions(-) >=20 > diff --git a/drivers/raw/ifpga_rawdev/base/opae_hw_api.c > b/drivers/raw/ifpga_rawdev/base/opae_hw_api.c > index a533dfe..50f6438 100644 > --- a/drivers/raw/ifpga_rawdev/base/opae_hw_api.c > +++ b/drivers/raw/ifpga_rawdev/base/opae_hw_api.c > @@ -303,12 +303,35 @@ static struct opae_adapter_ops *match_ops(struct=20 > opae_adapter *adapter) } >=20 > /** > - * opae_adapter_data_alloc - alloc opae_adapter_data data structure > + * opae_adapter_init - init opae_adapter data structure > + * @adpdate: pointer of opae_adater data structure > + * @name: adapter name. > + * @data: private data of this adapter. > + * > + * Return: 0 on success. > + */ > +int opae_adapter_init(struct opae_adapter *adapter, > + const char *name, void *data) > +{ > + if (!adapter) > + return -ENOMEM; > + > + TAILQ_INIT(&adapter->acc_list); > + adapter->data =3D data; > + adapter->name =3D name; > + adapter->ops =3D match_ops(adapter); > + > + return 0; > +} > + > +/** > + * opae_adapter_alloc - alloc opae_adapter data structure > * @name: adapter name. > * @data: private data of this adapter. > * > * Return: opae_adapter on success, otherwise NULL. > */ > + /**This function will no longer be called. > struct opae_adapter *opae_adapter_alloc(const char *name, void *data) { > struct opae_adapter *adapter =3D opae_zmalloc(sizeof(*adapter)); @@=20 > -316,13 +339,12 @@ struct opae_adapter *opae_adapter_alloc(const char=20 > *name, void *data) > if (!adapter) > return NULL; >=20 > - TAILQ_INIT(&adapter->acc_list); > - adapter->data =3D data; > - adapter->name =3D name; > - adapter->ops =3D match_ops(adapter); > + if (opae_adapter_init(adapter, name, data)) > + return NULL; >=20 > return adapter; > } To avoid redundancy, does this function useful? > +**/ >=20 > /** > * opae_adapter_enumerate - enumerate this adapter diff --git=20 > a/drivers/raw/ifpga_rawdev/base/opae_hw_api.h > b/drivers/raw/ifpga_rawdev/base/opae_hw_api.h > index 4bbc9df..ac1941a 100644 > --- a/drivers/raw/ifpga_rawdev/base/opae_hw_api.h > +++ b/drivers/raw/ifpga_rawdev/base/opae_hw_api.h > @@ -225,7 +225,9 @@ struct opae_adapter { void=20 > *opae_adapter_data_alloc(enum opae_adapter_type type); #define > opae_adapter_data_free(data) opae_free(data) >=20 > -struct opae_adapter *opae_adapter_alloc(const char *name, void=20 > *data); > +int opae_adapter_init(struct opae_adapter *adapter, > + const char *name, void *data); > +//struct opae_adapter *opae_adapter_alloc(const char *name, void=20 > +*data); > #define opae_adapter_free(adapter) opae_free(adapter) >=20 > int opae_adapter_enumerate(struct opae_adapter *adapter); diff --git=20 > a/drivers/raw/ifpga_rawdev/ifpga_rawdev.c > b/drivers/raw/ifpga_rawdev/ifpga_rawdev.c > index 32e318f..d433091 100644 > --- a/drivers/raw/ifpga_rawdev/ifpga_rawdev.c > +++ b/drivers/raw/ifpga_rawdev/ifpga_rawdev.c > @@ -409,9 +409,10 @@ > data->device_id =3D pci_dev->id.device_id; > data->vendor_id =3D pci_dev->id.vendor_id; >=20 > + adapter =3D rawdev->dev_private; > /* create a opae_adapter based on above device data */ > - adapter =3D opae_adapter_alloc(pci_dev->device.name, data); > - if (!adapter) { > + ret =3D opae_adapter_init(adapter, pci_dev->device.name, data); > + if (ret) { > ret =3D -ENOMEM; > goto free_adapter_data; Return error from opae_adapter_init() will cause free data? Do you need to free data? > } > @@ -420,8 +421,6 @@ > rawdev->device =3D &pci_dev->device; > rawdev->driver_name =3D pci_dev->device.driver->name; >=20 > - rawdev->dev_private =3D adapter; > - > /* must enumerate the adapter before use it */ > ret =3D opae_adapter_enumerate(adapter); > if (ret) > -- > 1.8.3.1