Received: by 2002:a05:6902:102b:0:0:0:0 with SMTP id x11csp3384954ybt; Tue, 30 Jun 2020 01:07:49 -0700 (PDT) X-Google-Smtp-Source: ABdhPJw5rNxdP7YQVy6ZnwrKTwsNrVUuyo8FttG2el49hE4Qu3Mt8c6WLTOHMDojqtl7/ztgIUaL X-Received: by 2002:a05:6402:6c4:: with SMTP id n4mr22433725edy.353.1593504469274; Tue, 30 Jun 2020 01:07:49 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1593504469; cv=none; d=google.com; s=arc-20160816; b=UCa/MsLisjpv0NykoR8c6T1Tlb3zjSHzptAhrSy8U7iUPpV8jEVx3bhW/9kdWMFVG1 VnWoirrtDKXNJaPyy4seMdg5mVwCVVX3GDhbLHMEjKlhAPbJ58mOWGNJ1MpDi5R6SMF5 mggGHH6PQOuITq+FR3z3Tk+L8gfqQ1qF+b89ljllEycmE+6LyS1Ubvcl50NB+BMw6CGo U44daZV+xWy2JY/2C3wIVShGqDTO5ZlG8mt1anA2D9sJjqmUgvUDfqmEJ8iHIOrTErVb IfpzOX/QLeImFBHbBr7d9p6AGXdNCbrTSPBTAAVxl1GKr9f1Wx2o74bgBDyv/S+WsBbx SvBw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:in-reply-to:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :dkim-signature; bh=jVFDX8+Dt0Ege3NTR18gZIgQHLkG2rp7n8jTwrZluS0=; b=UCVrbTNka4c/ojQOMXNXDjgQ0ZmLgVh6OanLIGN52zkJ0l4XXYld/Vms1G3HVKimXL /1X9OqwHZZD7GLhJla3G7odwxnKIbYm8SsiZurhZyNEqOKvzuZ2NZJSOvN6O43LaKL0o EZ3p9LKbuWRi08DQKEMnQBZpqEieLSoR8bFJCNxT2dkOsuGqtdwbmXZQtLnQyGUZm3IV bIL95dwreVff4R/ocpRYD5wo1IVKjCmlHko1dYR94ijaPRqxGOlJh6hCXIrCSRCAcg+o zCNqJYC8F1Bd5t7cxDIFSe7CYcUy96lh/QDf6u3rUVRHc4f8lTyceqaBCwiYui7dQEWs tSnw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=tYqMAa4N; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id a14si1263119ejv.483.2020.06.30.01.07.26; Tue, 30 Jun 2020 01:07:49 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=tYqMAa4N; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730850AbgF3IFt (ORCPT + 99 others); Tue, 30 Jun 2020 04:05:49 -0400 Received: from mail.kernel.org ([198.145.29.99]:43254 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731014AbgF3IFk (ORCPT ); Tue, 30 Jun 2020 04:05:40 -0400 Received: from localhost (unknown [84.241.197.94]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 5CF7620759; Tue, 30 Jun 2020 08:05:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1593504339; bh=wWSfqiUJtrzmxK9QO1B29WqsNSrAagcMWusAIHro7NQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=tYqMAa4Nd7/QQaLSl6ZDhsRRStDPBlHasRsgQqYmx4sAZmlT+ILY1dUldkLRD5US4 7pJVlXmVnl+mV/LwNxdkXhEAWQWYp2AWEgx1M5k9wAVWZPFY+MTMQx8bsGoMNkEILf RrgyRHwQkgVZDvHhe9PVJtWt1F7Bxp4EQab2rmMU= Date: Tue, 30 Jun 2020 10:05:35 +0200 From: Greg KH To: "Paraschiv, Andra-Irina" Cc: linux-kernel@vger.kernel.org, Anthony Liguori , Benjamin Herrenschmidt , Colm MacCarthaigh , Bjoern Doebel , David Woodhouse , Frank van der Linden , Alexander Graf , Martin Pohlack , Matt Wilson , Paolo Bonzini , Balbir Singh , Stefano Garzarella , Stefan Hajnoczi , Stewart Smith , Uwe Dannowski , kvm@vger.kernel.org, ne-devel-upstream@amazon.com Subject: Re: [PATCH v4 07/18] nitro_enclaves: Init misc device providing the ioctl interface Message-ID: <20200630080535.GD619174@kroah.com> References: <20200622200329.52996-1-andraprs@amazon.com> <20200622200329.52996-8-andraprs@amazon.com> <20200629162013.GA718066@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jun 29, 2020 at 08:45:25PM +0300, Paraschiv, Andra-Irina wrote: > > > On 29/06/2020 19:20, Greg KH wrote: > > > > On Mon, Jun 22, 2020 at 11:03:18PM +0300, Andra Paraschiv wrote: > > > +static int __init ne_init(void) > > > +{ > > > + struct pci_dev *pdev = pci_get_device(PCI_VENDOR_ID_AMAZON, > > > + PCI_DEVICE_ID_NE, NULL); > > > + int rc = -EINVAL; > > > + > > > + if (!pdev) > > > + return -ENODEV; > > Ick, that's a _very_ old-school way of binding to a pci device. Please > > just be a "real" pci driver and your probe function will be called if > > your hardware is present (or when it shows up.) To do it this way > > prevents your driver from being auto-loaded for when your hardware is > > seen in the system, as well as lots of other things. > > This check is mainly here in the case any codebase is added before the pci > driver register call below. What do you mean by "codebase"? You control this driver, just do all of the logic in the probe() function, no need to do this in the module init call. > And if we log any error with dev_err() instead of pr_err() before the driver > register. Don't do that. > That check was only for logging purposes, if done with dev_err(). I removed > the check in v5. Again, don't do it :) > > > > + > > > + if (!zalloc_cpumask_var(&ne_cpu_pool.avail, GFP_KERNEL)) > > > + return -ENOMEM; > > > + > > > + mutex_init(&ne_cpu_pool.mutex); > > > + > > > + rc = pci_register_driver(&ne_pci_driver); > > Nice, you did it right here, but why the above crazy test? > > > > > + if (rc < 0) { > > > + dev_err(&pdev->dev, > > > + "Error in pci register driver [rc=%d]\n", rc); > > > + > > > + goto free_cpumask; > > > + } > > > + > > > + return 0; > > You leaked a reference on that pci device, didn't you? Not good :( > > Yes, the pci get device call needs its pair - pci_dev_put(). I added it here > and for the other occurrences where it was missing. Again, just don't do it and then you don't have to worry about any of this. thanks, greg k-h