From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C390FCCA47C for ; Tue, 7 Jun 2022 05:49:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236806AbiFGFtB (ORCPT ); Tue, 7 Jun 2022 01:49:01 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53722 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230090AbiFGFtA (ORCPT ); Tue, 7 Jun 2022 01:49:00 -0400 Received: from verein.lst.de (verein.lst.de [213.95.11.211]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9A53BBA99B; Mon, 6 Jun 2022 22:48:56 -0700 (PDT) Received: by verein.lst.de (Postfix, from userid 2407) id 197A068BFE; Tue, 7 Jun 2022 07:48:53 +0200 (CEST) Date: Tue, 7 Jun 2022 07:48:52 +0200 From: Christoph Hellwig To: Kirti Wankhede Cc: Christoph Hellwig , Tony Krowiak , Halil Pasic , Jason Herne , Eric Farman , Matthew Rosato , Zhenyu Wang , Zhi Wang , Alex Williamson , kvm@vger.kernel.org, linux-s390@vger.kernel.org, intel-gvt-dev@lists.freedesktop.org, Neo Jia , Dheeraj Nigam , Tarun Gupta Subject: Re: [PATCH 2/8] vfio/mdev: embedd struct mdev_parent in the parent data structure Message-ID: <20220607054852.GA8680@lst.de> References: <20220603063328.3715-1-hch@lst.de> <20220603063328.3715-3-hch@lst.de> <71e7d9a8-1005-0458-b8cd-147ccc6430d7@nvidia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <71e7d9a8-1005-0458-b8cd-147ccc6430d7@nvidia.com> User-Agent: Mutt/1.5.17 (2007-11-01) Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On Tue, Jun 07, 2022 at 12:52:30AM +0530, Kirti Wankhede wrote: > By removing this list, there is no way to know if parent device is > registered repeatedly? What will happen if same parent device is registered > twice? will it fail somewhere else? The driver core will warn if you double register a device. >> probe'd to then it should call:: >> - extern int mdev_register_device(struct device *dev, >> - struct mdev_driver *mdev_driver); >> + int mdev_register_parent(struct mdev_parent *parent, struct device *dev, >> + struct mdev_driver *mdev_driver); >> > > No need to change API name as it still registers 'struct device *dev', it > could be 'mdev_register_device' but with new argument list. I think the name name is a lot more clear, as device is really overused. Especially as this is not a mdev_device, which are registered elsewhere.. >> - mdev_unregister_device(i915->drm.dev); >> + mdev_unregister_parent(&i915->vgpu.parent); > > Ideally, parent should be member of gvt. There could be multiple vgpus > created on one physical device. Intel folks would be better reviewer > though. i915->vgpu is not for a single VGPU, but all VGPU related core support. >> - struct mdev_parent *parent; >> char *env_string = "MDEV_STATE=registered"; >> char *envp[] = { env_string, NULL }; >> + int ret; >> /* check for mandatory ops */ >> if (!mdev_driver->supported_type_groups) >> return -EINVAL; >> - dev = get_device(dev); >> - if (!dev) >> - return -EINVAL; >> - > > Why not held device here? What if some driver miss behave where it > registers device to mdev, but unloads without unregistering from mdev? Then that driver is buggy. We don't add extra reference to slightly paper over buggy code elsewhere in the kernel either.