From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id D6535A0547; Mon, 22 Nov 2021 12:34:53 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 70E3040DF4; Mon, 22 Nov 2021 12:34:53 +0100 (CET) Received: from NAM10-MW2-obe.outbound.protection.outlook.com (mail-mw2nam10on2049.outbound.protection.outlook.com [40.107.94.49]) by mails.dpdk.org (Postfix) with ESMTP id 30DF840395 for ; Mon, 22 Nov 2021 12:34:52 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=n8qLNgA0skI85vUsHW/nEXlk+DGqyhF/ZR3gS2f4UUB3Fkc+W6PSbKpQr6q8yr+K/yo1GusGPgfT3lTZNm9uOSOIwi1Sf57Pl00vhqSY5HxIuiwLxJLZOUBOCQdlPci+nJ4IjlAhHz3HUHTjmUqH4Bb2z8UefVwiChgqaDaNZ31mj0W9RCMoXEACGzxyzg4cBzTlycu/A+GceTtvskD39REE196sRohfitV7v8L4Px22WqxPyUrNQA7dZGITnjy++J15KexxOr62nmQES8g6Y75DE+BFEvFAylzlyI32wenH40aBram8/vhOwt9M1jFJZfTCe1FP0y+lOqa1Ssuvjw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=RidTHu3oTyONWG8LhRfmJ07WTKS+lBPQmmg4yJMajME=; b=GqvMJVXVAALp5dphHUdd5DwzISuBFZ1mUBIbI0TnZWngv6CKHIsEkjF+POmTx12gYxSE9sJVZMjM340l8NtH8UUtloI75rS4x87KS9UemsNfkKL65eMShMm6Batd/bJMO7pDAUPYK3GcOdkmo9PDHPkv4dhgO9BqrRXngE0s5d7zcQJxCIiwuWRdtoegeUZ6U7btwmBcXzB5GMxsVvksN7HjDmsKyj170zYxwi+LKz0D9IafOR7MjUQgf/R4Qj5Vrg45ky7cQKOKy1b8sHZ1gi3qKBmyTjamo28fntXM6IXI7vmhRvKj1QeVAOGiAZFVeZM8QjressFTbS5myiz0Fw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nvidia.com; dmarc=pass action=none header.from=nvidia.com; dkim=pass header.d=nvidia.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Nvidia.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=RidTHu3oTyONWG8LhRfmJ07WTKS+lBPQmmg4yJMajME=; b=E8IZnNHd9u7XpUk7GDuelMXwNhMargPUBZQ1eVUnfyoR+chQnVeuewprd5G4iYYCJWatayr42/52IKqfwoCkD0PZaEPPQE+kxKetJwKv8rY04s8rvzBDofkf8hevjiJZ57jA7tXLWqMamqMJggAfmPptsltYZpqRNL7QNRfJJ+Jqr9U/CCfjDKNfCObVoeTokF8SsIg9d12mekMfMVVFjTAa0frzpHdVXy9h5XTJE0cb/SqdtxNP5fQu0FvdzD+ePlJjY5f66qjmaK0cJyVfG4T72ezkhEJyU3wHqolo9HGQS3hvAo4ZEvGp33FzpZOEOYuxbWT6el7YMQml8RJqrg== Received: from CH2PR12MB4104.namprd12.prod.outlook.com (2603:10b6:610:a4::12) by CH2PR12MB3864.namprd12.prod.outlook.com (2603:10b6:610:25::27) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4713.22; Mon, 22 Nov 2021 11:34:50 +0000 Received: from CH2PR12MB4104.namprd12.prod.outlook.com ([fe80::589f:6904:ad1e:f2b2]) by CH2PR12MB4104.namprd12.prod.outlook.com ([fe80::589f:6904:ad1e:f2b2%8]) with mapi id 15.20.4690.029; Mon, 22 Nov 2021 11:34:50 +0000 From: Elena Agostini To: "NBU-Contact-Thomas Monjalon (EXTERNAL)" CC: "dev@dpdk.org" Subject: Re: [PATCH v3] gpudev: manage NULL pointer Thread-Topic: [PATCH v3] gpudev: manage NULL pointer Thread-Index: AQHX34msl0WlFbYhRkCxXdH+cepL2qwPZ4+AgAAC+Ls= Date: Mon, 22 Nov 2021 11:34:50 +0000 Message-ID: References: <20211118203354.25355-1-eagostini@nvidia.com> <20211122182423.5581-1-eagostini@nvidia.com> <3103480.BfGS6plSxf@thomas> In-Reply-To: <3103480.BfGS6plSxf@thomas> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=nvidia.com; x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: dd6b2740-c415-4bc1-2d0f-08d9adac1853 x-ms-traffictypediagnostic: CH2PR12MB3864: x-ld-processed: 43083d15-7273-40c1-b7db-39efd9ccc17a,ExtAddr x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:9508; x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: SL7rJdQ14ebCCeBZQXXhNTt6RysvDIsWwwc+717CIB4CTTBma03MRLW9hgNXayEpcNhS89/0Yk6b+LyJFZO/Cn01HaAi233/KrZH63afXfP+nwbpzNUBLqfcGetpK9jd8Nxz4OK9r/HIjqz4pKiAOZsHa27dK37pQsSw/D7lg143nO6o6F/cqHirH9PijU/XGyvHPZihiRIy+83QeKJOZmg3V1Fo3CBldEllQlBqKir0oLP2YpzH9CiOMRGms5C2vpfP0uxLizIAt06BDe/xabieFpO2/Y+eDecdgmYON0I9c62r8yJFucZE2S/2xSDe7S6+sCIiv/EUFFcPV3UfYt2nr2lbF24MIVpcX2NN1+9dINi6sToEn7nPfkpMzMdWwEWAWoYEfC/l1TwyHvuJovfF4vRwRLCN6eQvizO4+tGQ7I6D6fmHpnTUyquwKGaNkxPi9O2vUF1nDo72F+e1TnIb3f8iTKG29nx5CVXblTiuMhVqDMhLSrivXaaDUKhSif4HqUQpRJmC98PyFbeoD6g8SatO0THCeCXh+UlM6sFXsN4suoll5UNC6ZIiq+3vNNIeqn9xBToYammBknqnoePcU6HHWYaklmLKWCL7Jpmw6LQFhQcT2LRdy9D73a9qP49Rj5hoai/ZIPJcg1qg6W6Y3TMKBB+ruZb7KTNWCdflygwQV0Ulj2aM0whfsfxnSyOilbQtQ19JG5kwYcsozw== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:CH2PR12MB4104.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(366004)(7696005)(33656002)(91956017)(5660300002)(6506007)(66556008)(66446008)(64756008)(186003)(66946007)(55016002)(316002)(26005)(6916009)(71200400001)(4326008)(76116006)(2906002)(66476007)(9686003)(122000001)(38070700005)(52536014)(8676002)(508600001)(86362001)(53546011)(8936002)(83380400001)(38100700002); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?tVvhuPBVtsgHfQHyfulLncZAH4jGPjWmVaDrIMkqPNC0S5L93FdkFn2B41cg?= =?us-ascii?Q?2zAdb3DnmUUVYdzlG/SGQvvT21RFdYl+6xdJTgj5e+flhnyRhFhIGQd56BpN?= =?us-ascii?Q?LHG03XCKcAuKqQd5Z5zu1kpMHOxsiL0tCfTu0hC9mikC0BHLfEoM69oRZEtr?= =?us-ascii?Q?Qaz0/3X2Ldd81yaZ488FJkLb82Le2Wsdxn2gLMbHoLjqL4Av+0YPpT/7sSpR?= =?us-ascii?Q?jUgHtFS+IW8i8yXizVOvuA8MhxPJ9afHj0cShHQqSqaPNVyKYGeptmFETXtH?= =?us-ascii?Q?pMFlooI0Y3SgR9m4oevYpgWaK1CSJ/QRUkZFWyJvEaUCYWNyTrOFOLykhrPQ?= =?us-ascii?Q?3XwvwZWNcwyYN3drGAfFuQr3iK1G5KAw1Rh3e2RXbWmoBpMBSBZ0GNt3wxXy?= =?us-ascii?Q?X/CaU6wIxza+dzwixsypxF9EnMOZJLxSY4AGRVmcZFvc16//1xfQiPGPXWhv?= =?us-ascii?Q?W4Hvrd2fiKNvGhUInx6219LSe+biZZo4N2I2BBEq2UStbYiCWLiTrn7couwI?= =?us-ascii?Q?zEzgCeq2sccHubRSMiZodRiRC9Bjnc0nNyVL9V1OaX9ICiROBBHrT4vFjNRn?= =?us-ascii?Q?M5YEoqYO6EVGfdCV4vLGA3M460qVDcLbruKuqSySYdfJLyn30BCClkGyWyGg?= =?us-ascii?Q?B0+NvmD+fUz2Q3iTXmBFnuy9TlCqTJLPKCN2FIgtcrVh64vXw6qMeoUGU6Jq?= =?us-ascii?Q?VqC/LrVIGEji2jS/xF2yx7TrNxSzD5bFqYvw4HAzSBuA48MNzIllIpNb+5ej?= =?us-ascii?Q?mEV6dZNI3Y4QiQBW3SA7SI5Diu42fJ3GIPGHdX3LV+Yyf2NXey7rptq91vAY?= =?us-ascii?Q?iEcXlSYxU6XPsK8v99YAMu65fl1E+w2yhwyBqLaQXIDfK91Tq9pY6hpSabAc?= =?us-ascii?Q?K/7skkpjqMdXUtQyeOTe+k4aoqvTqHC/u1QhTTvWVAW/h63JacSEoxXnD83u?= =?us-ascii?Q?L39jKpzl2BcfCoMw3AU/ZpD36wHaTmOb3pkUtPt2R7Y+k4dy1Sch47wtcwwt?= =?us-ascii?Q?ZkXGfgItgSteNkQIQ5p9MOo8TWUre6khSn9+1ZkNWdWcwBSetC+3WvrltnVI?= =?us-ascii?Q?4raUIUC7YcZp80x0P8ctFVHrNUsrFGLp9JMgKkR2v2XLVDCOMx0brcjYSNAB?= =?us-ascii?Q?BKRvVyJy44H5mVAre4cU68SwsB1fZ2D89cjEyXbYBs3pD08JzTXzsl802V4T?= =?us-ascii?Q?FdqIqjoKhPvCa8H34RyIdLaPnypTlNdI2ePgqZuJJqCF5Ngzx76VGUlR4Q6n?= =?us-ascii?Q?gKbJn6WDWNjyg57sq2gTRfU6GswjvpaYJoimZ66WYLW4DrtMeKxaSav3ydLl?= =?us-ascii?Q?QUmL3ZDhK9mVJ/J2kYne9pxxuv4SrFhYNrmd5zuhab+i8cWxfjzKy/rV7XBm?= =?us-ascii?Q?SitNqc+EZmswoZpzdOxyce//Yp7OBzeIADR4UEym/JzTgbv8bGgf07+9AVaR?= =?us-ascii?Q?1yVDy/ypT3N0WRdv5KX1KLE/hW/eJcH8ITWR5wWd8EKK3ct+s93sroMHy/G+?= =?us-ascii?Q?aHE+HD6I+OU0ZbjYNxOYvEV3RDHWlVN2XklQtrgrsE9q2q/tQ3hldTNpbZdW?= =?us-ascii?Q?CDZF9gcfltk5rKknZxlZTGm8ybZ0MfKqfZ8b0gpSgkJo1OzSnaCsWU7vv0wU?= =?us-ascii?Q?cOQLphMlsa0+6hI+1xOmNTZw+omYfRmATumnApFBBBeC?= Content-Type: multipart/alternative; boundary="_000_CH2PR12MB410474F4B3B064D1CF10D50CCD9F9CH2PR12MB4104namp_" MIME-Version: 1.0 X-OriginatorOrg: Nvidia.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: CH2PR12MB4104.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: dd6b2740-c415-4bc1-2d0f-08d9adac1853 X-MS-Exchange-CrossTenant-originalarrivaltime: 22 Nov 2021 11:34:50.2933 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 43083d15-7273-40c1-b7db-39efd9ccc17a X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: EVuqmhiEMhgoEeVH1u2dayA4UBNWWc05xMGNqzk54mKVU/+rPuEAbInN1LZwhc5zgUFEid4KeqctDCv0R3xdhg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR12MB3864 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org --_000_CH2PR12MB410474F4B3B064D1CF10D50CCD9F9CH2PR12MB4104namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > From: Thomas Monjalon > Date: Monday, 22 November 2021 at 12:23 > To: Elena Agostini > Cc: dev@dpdk.org > Subject: Re: [PATCH v3] gpudev: manage NULL pointer > External email: Use caution opening links or attachments> > > 22/11/2021 19:24, eagostini@nvidia.com: > > From: Elena Agostini > > > > gpudev free and unregister functions return gracefully if input pointer= is NULL> > We could add that the API doc was indicating NULL as a no-op accepted val= ue.> > Another explanation to add: cuda driver checks are removed because redund= ant > with the checks added in gpudev library.> > > Fixes: 818a067baf90 ("gpudev: manage NULL pointer")> > It should be: > Fixes: e818c4e2bf50 ("gpudev: add memory API")> > > > > Signed-off-by: Elena Agostini > > --- > > drivers/gpu/cuda/cuda.c | 6 ------ > > lib/gpudev/gpudev.c | 6 ++++++ > > 2 files changed, 6 insertions(+), 6 deletions(-) > [...] > > --- a/lib/gpudev/gpudev.c > > +++ b/lib/gpudev/gpudev.c > > @@ -569,6 +569,9 @@ rte_gpu_mem_free(int16_t dev_id, void *ptr) > > { > > struct rte_gpu *dev; > > > > + if (ptr =3D=3D NULL) > > + return 0; > > + > > dev =3D gpu_get_by_id(dev_id); > > if (dev =3D=3D NULL) { > > GPU_LOG(ERR, "free mem for invalid device ID %d", dev_id)= ;> > I think we should keep this check first. Why should gpudev waste more latency in looking for the device if the ptr i= s NULL? > > @@ -612,6 +615,9 @@ rte_gpu_mem_unregister(int16_t dev_id, void *ptr) > > { > > struct rte_gpu *dev; > > > > + if (ptr =3D=3D NULL) > > + return 0; > > + > > dev =3D gpu_get_by_id(dev_id); > > if (dev =3D=3D NULL) { > > GPU_LOG(ERR, "unregister mem for invalid device ID %d", d= ev_id);> > Same here. > There is third function where NULL should be accepted: rte_gpu_mem_regist= er> Thanks, let me add it --_000_CH2PR12MB410474F4B3B064D1CF10D50CCD9F9CH2PR12MB4104namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable

> From: Thomas M= onjalon <thomas@monjalon.net>

> Date: Monday, = 22 November 2021 at 12:23

> To: Elena Agos= tini <eagostini@nvidia.com>

> Cc: dev@dpdk.o= rg <dev@dpdk.org>

> Subject: Re: [= PATCH v3] gpudev: manage NULL pointer

> External email= : Use caution opening links or attachments>

>

 

> 22/11/2021 19:= 24, eagostini@nvidia.com:

> > From: Ele= na Agostini <eagostini@nvidia.com>

> >

> > gpudev fr= ee and unregister functions return gracefully if input pointer is NULL>

 

> We could add t= hat the API doc was indicating NULL as a no-op accepted value.>

 

> Another explan= ation to add: cuda driver checks are removed because redundant

> with the check= s added in gpudev library.>

 

> > Fixes: 81= 8a067baf90 ("gpudev: manage NULL pointer")>

 

> It should be:<= o:p>

> Fixes: e818c4e= 2bf50 ("gpudev: add memory API")>

 

> >

> > Signed-of= f-by: Elena Agostini <eagostini@nvidia.com>

> > ---<= /o:p>

> >  dri= vers/gpu/cuda/cuda.c | 6 ------

> >  lib= /gpudev/gpudev.c     | 6 ++++++

> >  2 f= iles changed, 6 insertions(+), 6 deletions(-)

> [...]

> > --- a/lib= /gpudev/gpudev.c

> > +++ b/lib= /gpudev/gpudev.c

> > @@ -569,6= +569,9 @@ rte_gpu_mem_free(int16_t dev_id, void *ptr)

> >  {

> > &nbs= p;     struct rte_gpu *dev;

> >

> > + &n= bsp;   if (ptr =3D=3D NULL)

> > + &n= bsp;           return 0;<= o:p>

> > +

> > &nbs= p;     dev =3D gpu_get_by_id(dev_id);=

> > &nbs= p;     if (dev =3D=3D NULL) {

> > &nbs= p;             = GPU_LOG(ERR, "free mem for invalid device ID %d", dev_id);>

 

> I think we sho= uld keep this check first.

 

Why should gpudev w= aste more latency in looking for the device if the ptr is NULL?<= /span>

 

> > @@ -612,6= +615,9 @@ rte_gpu_mem_unregister(int16_t dev_id, void *ptr)

> >  {

> > &nbs= p;     struct rte_gpu *dev;

> >

> > + &n= bsp;   if (ptr =3D=3D NULL)

> > + &n= bsp;           return 0;<= o:p>

> > +

> > &nbs= p;     dev =3D gpu_get_by_id(dev_id);=

> > &nbs= p;     if (dev =3D=3D NULL) {

> > &nbs= p;             = GPU_LOG(ERR, "unregister mem for invalid device ID %d", dev_id);&= gt;

 

> Same here.

 

> There is third= function where NULL should be accepted: rte_gpu_mem_register>

 

Thanks, let me add = it

--_000_CH2PR12MB410474F4B3B064D1CF10D50CCD9F9CH2PR12MB4104namp_--