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 A4582A0547; Mon, 22 Nov 2021 12:51:51 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2544B40395; Mon, 22 Nov 2021 12:51:51 +0100 (CET) Received: from out4-smtp.messagingengine.com (out4-smtp.messagingengine.com [66.111.4.28]) by mails.dpdk.org (Postfix) with ESMTP id 8A61E4014E for ; Mon, 22 Nov 2021 12:51:49 +0100 (CET) Received: from compute6.internal (compute6.nyi.internal [10.202.2.46]) by mailout.nyi.internal (Postfix) with ESMTP id F376E5C0114; Mon, 22 Nov 2021 06:51:48 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute6.internal (MEProxy); Mon, 22 Nov 2021 06:51:48 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=fm2; bh= 68MGUpi/RwwQdszRRT/RmD7yZrzPzAnHs/cL0QrS0XQ=; b=JiXwqZXWTps56CLv lGZy63evUEtns9fweHeJffpaXXVVucjLq8bx82XeT58eXkVCEKIU7gjYGtm4k+Y+ 2O9KZUqMtchOn8dgU/o6t7tiscSHHnQGWBgh97NwpXgcZ8QKkIxG9EhDXmAslQ5M lp6wBUPxQAKkpYq+VpCBZIema9usg4XKSCtDphV3MeOZEa35IrhozPMTVNtD49O3 l7VecxmEQwsIaCOUguYu6Owblod9+FSjEFVOsICNeeUKkrkTlFQrOtPhV3fEKyvB xIG9ps0Mwj9ZMLfhryySqzNq8w1W7aqdvL8sZCkT5Z83F4Ugymjvece3znCWluMR xxeywQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=68MGUpi/RwwQdszRRT/RmD7yZrzPzAnHs/cL0QrS0 XQ=; b=SClA6wJ7G4y0385QPtYyOHhVl3A/FIzYiPB9xPdC8o8jQ6+l1yn5OqiOp MqXBjjb7EwM49EWWRrVdPwXaXpAUcQvcsFhzrQF7bQf/6iWzdJYaP8QtjMMcPXPA cE20s3Wqty0MLhe1A3Yz1bb9Xd3/YsAXAZ0Kv63iZyFGAqyxyqszj1CNK6emDRwC KWLFeQvOjYT3nhlN4u+N8YT2PEzhDuvenA5XmHuZoBOYJnR3c82RSMhfEe2wdliG SF9gmIyTkAB5VuqNcAB3+WQaojUxLIHJ8QMDRxVnJW6M9VKenuKpfshN+wFE6ZI2 iTa3xLDj5IhOWzOKpnViRhEoYixcQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvuddrgeeggdefgecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhmrghs ucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucggtf frrghtthgvrhhnpedugefgvdefudfftdefgeelgffhueekgfffhfeujedtteeutdejueei iedvffegheenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhroh hmpehthhhomhgrshesmhhonhhjrghlohhnrdhnvght X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 22 Nov 2021 06:51:48 -0500 (EST) From: Thomas Monjalon To: Elena Agostini Cc: dev@dpdk.org Subject: Re: [PATCH v3] gpudev: manage NULL pointer Date: Mon, 22 Nov 2021 12:51:46 +0100 Message-ID: <4322799.Gvt5RlC7if@thomas> In-Reply-To: References: <20211118203354.25355-1-eagostini@nvidia.com> <3103480.BfGS6plSxf@thomas> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" 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 22/11/2021 12:34, Elena Agostini: > From: Thomas Monjalon > > 22/11/2021 19:24, eagostini@nvidia.com: > > > --- 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 == NULL) > > > + return 0; > > > + > > > dev = gpu_get_by_id(dev_id); > > > if (dev == 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 is NULL? Freeing with NULL pointer is not in the datapath I think, probably just a failure cleanup case. Having the dev_id check first allows to catch more bugs. Returning 0 without checking the id looks weird to me.