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;
Date:   Wed, 3 Nov 2021 10:25:47 -0300
From:   Jason Gunthorpe <jgg@nvidia.com>
To:     "Liu, Yi L" <yi.l.liu@intel.com>
Cc:     "alex.williamson@redhat.com" <alex.williamson@redhat.com>,
        "hch@lst.de" <hch@lst.de>,
        "jasowang@redhat.com" <jasowang@redhat.com>,
        "joro@8bytes.org" <joro@8bytes.org>,
        "jean-philippe@linaro.org" <jean-philippe@linaro.org>,
        "Tian, Kevin" <kevin.tian@intel.com>,
        "parav@mellanox.com" <parav@mellanox.com>,
        "lkml@metux.net" <lkml@metux.net>,
        "pbonzini@redhat.com" <pbonzini@redhat.com>,
        "lushenming@huawei.com" <lushenming@huawei.com>,
        "eric.auger@redhat.com" <eric.auger@redhat.com>,
        "corbet@lwn.net" <corbet@lwn.net>,
        "Raj, Ashok" <ashok.raj@intel.com>,
        "yi.l.liu@linux.intel.com" <yi.l.liu@linux.intel.com>,
        "Tian, Jun J" <jun.j.tian@intel.com>, "Wu, Hao" <hao.wu@intel.com>,
        "Jiang, Dave" <dave.jiang@intel.com>,
        "jacob.jun.pan@linux.intel.com" <jacob.jun.pan@linux.intel.com>,
        "kwankhede@nvidia.com" <kwankhede@nvidia.com>,
        "robin.murphy@arm.com" <robin.murphy@arm.com>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
        "iommu@lists.linux-foundation.org" <iommu@lists.linux-foundation.org>,
        "dwmw2@infradead.org" <dwmw2@infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "baolu.lu@linux.intel.com" <baolu.lu@linux.intel.com>,
        "david@gibson.dropbear.id.au" <david@gibson.dropbear.id.au>,
        "nicolinc@nvidia.com" <nicolinc@nvidia.com>
Subject: Re: [RFC 02/20] vfio: Add device class for /dev/vfio/devices
Message-ID: <20211103132547.GM2744544@nvidia.com>
References: <PH0PR11MB56583D477B3977D92C2C1ADDC3839@PH0PR11MB5658.namprd11.prod.outlook.com>
 <20211025125309.GT2744544@nvidia.com>
 <PH0PR11MB56586D2EC89F282C915AF18DC3879@PH0PR11MB5658.namprd11.prod.outlook.com>
 <20211101125013.GL2744544@nvidia.com>
 <PH0PR11MB565808A9C9974A0D0D72B738C38B9@PH0PR11MB5658.namprd11.prod.outlook.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <PH0PR11MB565808A9C9974A0D0D72B738C38B9@PH0PR11MB5658.namprd11.prod.outlook.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?k42aeq/H0HCnICeZYpDLQ1WdbhX3ulAy42JPq08wUbYJ51/EBX3/nQT7ZHOk?=
 =?us-ascii?Q?fKurEs+jr/fX75SHbiDvtMWYG475S6SyesvT+LSIfY+WYw6IX9B6rNTCUtCF?=
 =?us-ascii?Q?MSuO+hb+mbgdtPxty+bczXyyDda10ZRcr+YiC9Ruvj/Yy6BOJQGWUR70P+Nf?=
 =?us-ascii?Q?g2c6TzRdAJDJEQyFBNxn44rgBLM4H+er7Bi4KTPClW++jw4wIuhsfAEtHhrT?=
 =?us-ascii?Q?TG/OeD48oKzLBzBCsjKuF/p8JYOkv/6UBWwbRfuRRq8p8IKOdeKLsgO4JjYV?=
 =?us-ascii?Q?i8y5Ag1rby5i0edNg8TrkuKblHAEJybEIKNMIFOvOLRg/K3frvHETJVAhKPv?=
 =?us-ascii?Q?PyvswUWI1goFLuc/kBfeUbYXD/BUwcmOixgsVHbQ4QWKAJsRrDqnsvAZMaAa?=
 =?us-ascii?Q?vuVnzdMNF19dbcwo8UYzraegR0YIQNxleHJ816TWGD+PN/oApQNpYnMTebQH?=
 =?us-ascii?Q?QMFxQhn7wU6O1l0eoZdWhoA+6+zrcoGY3doSzOUITqpp8DGlvM4OqRzq0Z6b?=
 =?us-ascii?Q?Ykqu4ONiyH2xM4sLFM4kcoXYFcrmJZw+TPoEjMnX51FaBVkPNZKoAEXyi3b0?=
 =?us-ascii?Q?6/3ka511j+nLVd24uK5pxoLsJBZ1i7pimFALbfgf3Bk6Ok8CKzL/1/jastih?=
 =?us-ascii?Q?drb+0Dsr0CH32P3LFTNWza7qqJL/Ozq/qESHI8ZiWNSYaKYR2lrQN2IPt37+?=
 =?us-ascii?Q?l0T3edb9kOZlHkqwanqHm/oD6MxsP6/piN/FcyCOlwps1LwtgdoQqT2AWTL+?=
 =?us-ascii?Q?i9AxqyMl4rcUyVHjKCONowEpgAxDk7lGoW0XukCLNDuU4cSURUmQf8PqIVMi?=
 =?us-ascii?Q?EmRKN8t4crN8atkK8PMkNvrOhWnRkcecy5T4ytpN0eBrzZA56fE/OyfZN0ad?=
 =?us-ascii?Q?tk21rdd3342RJLLiRr98BAbUdEBHlUQjmOwTv8nM/j1Anr1y/RMrGOXlKX0j?=
 =?us-ascii?Q?KogLxDKTqv72mGgYe1RGNIMQ1D52tp36pXV0Wtf8jzwhc5Gxz9LnIFeYnbuk?=
 =?us-ascii?Q?jYcYH+hWHFd+rhh5iIBca+tNJkIbjfuMstE0plHFznPJVWccZDlVGmxTLE8g?=
 =?us-ascii?Q?QVxOK9ym0m1n1q20b2iuALiZZlXynef2FM5KZYZW8SE/I6XtK80T165unSsa?=
 =?us-ascii?Q?eYilyLzKET+2ZOII4qUWRIgrChdMtYO0l8zUNmxGZvtsiQuXbCzcfLy9Lb/V?=
 =?us-ascii?Q?neTguz6/oN7Cop1crRPrikocSw4/SxmeT5FvmoJcg5bKTyqq79GU1Enn/nLe?=
 =?us-ascii?Q?SHYgDyEuolsdh+bAPVpMoWEx/tqQJ2/ZJz78rpTCVaEu9B7iuDY3vuN6GL7B?=
 =?us-ascii?Q?dQTFwATZJX300DkkoMvAtLzL5nzGmCH+HMYFEWiC6AajRAq54nHJiE6fln9w?=
 =?us-ascii?Q?U5YBwHqVXPzYbYF4O76+lbkD2JfW5wP6KtB/knjzEXyBHvHL93HkCqxe3KMG?=
 =?us-ascii?Q?0q8OSLKHyTJaxFUB1t1vOBWuKE2kr7FqZ4mLkt/PFUezSCGEpORIH973cK4U?=
 =?us-ascii?Q?tLtMiXUaR3nf6x0DyuzztiOaU795+lF0xuwheFa9Rb55dVDV677QbUZP7P8g?=
 =?us-ascii?Q?LcECQOl9mtKrrhJaEn0=3D?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f67a8a9e-b58f-4938-db1b-08d99ecd7330
X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB5520.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Nov 2021 13:25:49.2421
 (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: WmoHTOIendl7eCU1V1s/USlrv3DHs+DFLn9znv2ZvIFaaGDdtcvcuqD3YV9JXbcc
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB5198
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, Nov 02, 2021 at 09:53:29AM +0000, Liu, Yi L wrote:

> > 	vfio_uninit_group_dev(&mdev_state->vdev);
> > 	kfree(mdev_state->pages);
> > 	kfree(mdev_state->vconfig);
> > 	kfree(mdev_state);
> > 
> > pages/vconfig would logically be in a release function
> 
> I see. So the criteria is: the pointer fields pointing to a memory buffer
> allocated by the device driver should be logically be free in a release
> function. right? 

Often yes, that is usually a good idea

>I can see there are such fields in struct vfio_pci_core_device
> and mdev_state (both mbochs and mdpy). So we may go with your option #2.
> Is it? otherwise, needs to add release callback for all the related drivers.

Yes, that is the approx trade off

> > On the other hand ccw needs to rcu free the vfio_device, so that would
> > have to be global overhead with this api design.
> 
> not quite get. why ccw is special here? could you elaborate?

I added a rcu usage to it in order to fix a race

+static inline struct vfio_ccw_private *vfio_ccw_get_priv(struct subchannel *sch)
+{
+       struct vfio_ccw_private *private;
+
+       rcu_read_lock();
+       private = dev_get_drvdata(&sch->dev);
+       if (private && !vfio_device_try_get(&private->vdev))
+               private = NULL;
+       rcu_read_unlock();
+       return private;
+}
 

Jason