From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id BDE7458F6 for ; Mon, 5 Nov 2018 09:25:34 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 05 Nov 2018 00:25:33 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,467,1534834800"; d="scan'208";a="278376980" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by fmsmga006.fm.intel.com with ESMTP; 05 Nov 2018 00:25:33 -0800 Received: from hasmsx107.ger.corp.intel.com (10.184.198.27) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.408.0; Mon, 5 Nov 2018 00:25:33 -0800 Received: from hasmsx105.ger.corp.intel.com ([169.254.1.193]) by hasmsx107.ger.corp.intel.com ([169.254.2.159]) with mapi id 14.03.0415.000; Mon, 5 Nov 2018 10:25:30 +0200 From: "Stojaczyk, Dariusz" To: Thomas Monjalon CC: "dev@dpdk.org" Thread-Topic: [PATCH 2/3] devargs: delay freeing previous devargs when overriding them Thread-Index: AQHUdNZqItA1WbJDPEKXMtpCJBnNIqVAp6iAgAAsIYA= Date: Mon, 5 Nov 2018 08:25:30 +0000 Message-ID: References: <20181105070447.67700-1-dariusz.stojaczyk@intel.com> <20181105070447.67700-2-dariusz.stojaczyk@intel.com> <4545148.O9CyC7tLm6@xps> In-Reply-To: <4545148.O9CyC7tLm6@xps> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNWIzNjVlYWItZTlkMC00NmU3LWEwYjEtYTQzNTBjMmJmMThlIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiRVpRZVplK2dpajRMQWpzU3hPZmJnMXJ2ZTBxdFlvN2k0STlqNHY1ckVOWEhMQ3BObTBcL0NHTGdWVXNzdjd1cTEifQ== x-ctpclassification: CTP_NT x-originating-ip: [10.102.11.35] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH 2/3] devargs: delay freeing previous devargs when overriding them 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: Mon, 05 Nov 2018 08:25:35 -0000 Hi Thomas, > -----Original Message----- > From: Thomas Monjalon [mailto:thomas@monjalon.net] > Sent: Monday, November 5, 2018 8:31 AM > To: Stojaczyk, Dariusz > Cc: dev@dpdk.org > Subject: Re: [PATCH 2/3] devargs: delay freeing previous devargs when > overriding them >=20 > Hi, >=20 > 05/11/2018 08:04, Darek Stojaczyk: > > -int __rte_experimental > > -rte_devargs_insert(struct rte_devargs *da) > > +void __rte_experimental > > +rte_devargs_insert(struct rte_devargs *da, struct rte_devargs > **prev_da) >=20 > You should update the API section of the release notes. Even for experimental API? OK, I didn't know it's needed. >=20 > > { > > - int ret; > > + struct rte_devargs *d; > > + void *tmp; > > + > > + *prev_da =3D NULL; > > + TAILQ_FOREACH_SAFE(d, &devargs_list, next, tmp) { > > + if (strcmp(d->bus->name, da->bus->name) =3D=3D 0 && > > + strcmp(d->name, da->name) =3D=3D 0) { > > + TAILQ_REMOVE(&devargs_list, d, next); > > + *prev_da =3D d; > > + break; > > + } > > + } > > > > - ret =3D rte_devargs_remove(da); > > - if (ret < 0) > > - return ret; >=20 > Why not updating rte_devargs_remove instead of duplicating its code? >=20 We still want to preserve the functionality of rte_devargs_remove. rte_devargs_remove does TAILQ_REMOVE + free; rte_devargs_insert does just T= AILQ_REMOVE. (I think I also forgot to update rte_devargs_insert documentat= ion, I'll do that in V2) Since you've mentioned it: Eventually I'd see rte_devargs_remove to accept the exact same devargs para= meter that was passed to rte_devargs_insert. Then rte_devargs_remove wouldn= 't do any sort of lookup. Maybe additional rte_devargs_find(const char *nam= e) could be added for existing cases where the original devargs struct is n= ot available. However, I'm not familiar enough with this code to perform th= e refactor and am just trying to fix the stuff. Still, how does it sound? D.