From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40736) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c4HOu-0001oQ-9W for qemu-devel@nongnu.org; Tue, 08 Nov 2016 20:10:05 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c4HOp-0002Cy-7E for qemu-devel@nongnu.org; Tue, 08 Nov 2016 20:10:04 -0500 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:60847 helo=mx0a-001b2d01.pphosted.com) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c4HOp-0002Cs-2M for qemu-devel@nongnu.org; Tue, 08 Nov 2016 20:09:59 -0500 Received: from pps.filterd (m0098421.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.17/8.16.0.17) with SMTP id uA918qYq142661 for ; Tue, 8 Nov 2016 20:09:58 -0500 Received: from e36.co.us.ibm.com (e36.co.us.ibm.com [32.97.110.154]) by mx0a-001b2d01.pphosted.com with ESMTP id 26kq6uys98-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Tue, 08 Nov 2016 20:09:58 -0500 Received: from localhost by e36.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 8 Nov 2016 18:09:57 -0700 Date: Wed, 9 Nov 2016 09:09:50 +0800 From: Dong Jia Shi References: <1478293856-8191-1-git-send-email-kwankhede@nvidia.com> <1478293856-8191-2-git-send-email-kwankhede@nvidia.com> <20161108092552.GA2090@bjsdjshi@linux.vnet.ibm.com> <8da9a274-281e-0804-0313-3d54d05ce0ad@nvidia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <8da9a274-281e-0804-0313-3d54d05ce0ad@nvidia.com> Message-Id: <20161109010950.GA4716@bjsdjshi@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH v11 01/22] vfio: Mediated device Core driver List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kirti Wankhede Cc: alex.williamson@redhat.com, pbonzini@redhat.com, kraxel@redhat.com, cjia@nvidia.com, qemu-devel@nongnu.org, kvm@vger.kernel.org, kevin.tian@intel.com, jike.song@intel.com, bjsdjshi@linux.vnet.ibm.com, linux-kernel@vger.kernel.org * Kirti Wankhede [2016-11-09 02:36:12 +0530]: [...] > >> +/* > >> + * mdev_register_device : Register a device > >> + * @dev: device structure representing parent device. > >> + * @ops: Parent device operation structure to be registered. > >> + * > >> + * Add device to list of registered parent devices. > >> + * Returns a negative value on error, otherwise 0. > >> + */ > >> +int mdev_register_device(struct device *dev, const struct parent_ops *ops) > >> +{ > >> + int ret; > >> + struct parent_device *parent; > >> + > >> + /* check for mandatory ops */ > >> + if (!ops || !ops->create || !ops->remove || !ops->supported_type_groups) > >> + return -EINVAL; > >> + > >> + dev = get_device(dev); > >> + if (!dev) > >> + return -EINVAL; > >> + > >> + mutex_lock(&parent_list_lock); > >> + > >> + /* Check for duplicate */ > >> + parent = __find_parent_device(dev); > >> + if (parent) { > >> + ret = -EEXIST; > >> + goto add_dev_err; > >> + } > >> + > >> + parent = kzalloc(sizeof(*parent), GFP_KERNEL); > >> + if (!parent) { > >> + ret = -ENOMEM; > >> + goto add_dev_err; > >> + } > >> + > >> + kref_init(&parent->ref); > >> + mutex_init(&parent->lock); > >> + > >> + parent->dev = dev; > >> + parent->ops = ops; > >> + > >> + if (!mdev_bus_compat_class) { > >> + mdev_bus_compat_class = class_compat_register("mdev_bus"); > >> + if (!mdev_bus_compat_class) { > >> + ret = -ENOMEM; > >> + goto add_dev_err; > >> + } > >> + } > >> + > >> + ret = parent_create_sysfs_files(parent); > >> + if (ret) > >> + goto add_dev_err; > >> + > >> + ret = class_compat_create_link(mdev_bus_compat_class, dev, NULL); > >> + if (ret) > >> + dev_warn(dev, "Failed to create compatibility class link\n"); > >> + > >> + list_add(&parent->next, &parent_list); > >> + mutex_unlock(&parent_list_lock); > >> + > >> + dev_info(dev, "MDEV: Registered\n"); > >> + return 0; > >> + > >> +add_dev_err: > >> + mutex_unlock(&parent_list_lock); > >> + if (parent) > >> + mdev_put_parent(parent); > > Why do this? I don't find the place that you call mdev_get_parent above. > > > > kref_init(&parent->ref); > Above increments the ref_count, so mdev_put_parent() should be called if > anything fails. > > >> + else > >> + put_device(dev); > > Shouldn't we always do this? > > > > When mdev_put_parent() is called, its release function do this. So if > mdev_put_parent() is called, we don't need this. Sorry for missing that. Thanks for the explanation! > > >> + return ret; > >> +} > >> +EXPORT_SYMBOL(mdev_register_device); > >> + [...] -- Dong Jia