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:   Mon, 25 Oct 2021 09:53:09 -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: <20211025125309.GT2744544@nvidia.com>
References: <PH0PR11MB56583D477B3977D92C2C1ADDC3839@PH0PR11MB5658.namprd11.prod.outlook.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <PH0PR11MB56583D477B3977D92C2C1ADDC3839@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?wq3VJHJo79XhpsZxOGRP7tL5FLvLPzacmM/QTLmQU1SDTc0S1RLipwxJP1p7?=
 =?us-ascii?Q?h9xawknUbBHvevaYQgMBQeohUzOSGnKHGhyC8Av2dByjJ0RuSvXeXpOxb0eH?=
 =?us-ascii?Q?TrhPB+rn2lt/c99D++Fp8PtnW4a5+lpnErjAXPKosJTNFRwsnsgXR9gP0ao6?=
 =?us-ascii?Q?Tw4kC1mPjQwJV3SDTgWRRkjjqh4+SiI0uxHzWSwSeqSmEgtYCPqxanWdJgLx?=
 =?us-ascii?Q?MvMUwI9EyUZab/eG2jjgi/aOaGt/3fGMXYDoTNAFDYfTxE4JYexVlaZQWiZw?=
 =?us-ascii?Q?ro/68xy3O+ShD4BDOGZZd5iKK1Xuh3ojTp0uX98vPQv+rj0XmOzPwvfsZBNp?=
 =?us-ascii?Q?DMx0UkvoP8hVz2vVzCSva3UG9W1QUiW+33P1hoJzSjN5at3nnCC4hx+AYHiJ?=
 =?us-ascii?Q?Wkyth4yApJtM1LlzCnj4aFLO9V34it3FYS1BUrFSR9iIxO6Y1lBxlVNl7Y0s?=
 =?us-ascii?Q?IWXZcpjHoakRJK0ZLNc7CSgU89nbojK+Sb/k0ez9vjYFuS69lzzzOAPEj+o2?=
 =?us-ascii?Q?sHBWbuNpGIjDnBv3vIoNrduluzelk2U+T4yVwd7h0/OukGUnFX+qHql1OMop?=
 =?us-ascii?Q?7VfWIIcHNhPEG3t6E9jXV6ocMRRFPFLvlD55XSwXcO4G8djn1tURumybGg14?=
 =?us-ascii?Q?bmw87AqM1SXUW9U67d6C7MUgCOyzFOuen08KDxyqW5Eq0H02ynWg9wtkWPEJ?=
 =?us-ascii?Q?FxAK1pbBircZIr5TGV7syXWGva4aD97C3oCZprleTkkLMj+YhfViOFQyPeHh?=
 =?us-ascii?Q?gQ9/iPnPa2QkCdKNz+CyFiGlCiE/32W9HCUQxtN/y0a21yjVCSxdu+keP3Dg?=
 =?us-ascii?Q?Ac72lf9uYLrQfEb/t4mrS4zf8p3bBgXLMZ2QrdW64hI7SZlB29N7iEyu6CVb?=
 =?us-ascii?Q?hDhN6Ff43nGE4UtiIuT3VpdpPwU4M9vKOdlXqqKO2SsuJzQEYCHeJP/KYyNd?=
 =?us-ascii?Q?WGiSOkwe66Nv6gz4uQYyedxC7rIqaOkzMqlDfcLfecau9mG/cCsoKeKYDERU?=
 =?us-ascii?Q?onr/9FqiGOm2ixOK7ytWdfUK2Rg4F9bPHQH7QbGWF5xixPWYb9bKGhwVTi64?=
 =?us-ascii?Q?jriTYI5DAjI2ECHMNHKGivfcclcf5o+wKbyJurlCnVJUl9AMMODDzi9dF55W?=
 =?us-ascii?Q?EUSEjukWP6GWwYDyfoulJMCzd+DosBsRYc9I/KeL8j4t789TYAJxZH34vA09?=
 =?us-ascii?Q?n5fN+AeEJ0VtAqXgSeajZjTww3dv6QjK2Mg6uWytWOBVdJrJjt7UWsTuEopf?=
 =?us-ascii?Q?G3fELlEYBtGwqvFOCikB+towV75CVo6RNLRxACYZOW10mCmC6LfTXpf4X1/Z?=
 =?us-ascii?Q?R6PiPfsUzNPdygNbjDnOhM3SVoGs0s1xA4qPK8uMJGa04vPFO1K56HTv605N?=
 =?us-ascii?Q?tv9ieoW68pmvCBVuHFIhrZ98Hmi9BCEl0HLG5h2bZrSCM3YG52x+FANJj1Pe?=
 =?us-ascii?Q?mQmwekj6+gVNbokrUwGavuI4RsIASIRiYZJaqr6k9Ig3OnGMpQY1l/chJbVY?=
 =?us-ascii?Q?K5L2IH4TV4dua3nuW509/CW9ZaBFhMlX3HHizZ+DHZmIAEleB7INGsP+TiZw?=
 =?us-ascii?Q?cenwxa76GnlzwPddOmM=3D?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c8743aee-27af-490f-1f4c-08d997b6670f
X-MS-Exchange-CrossTenant-AuthSource: BL0PR12MB5506.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Oct 2021 12:53:12.1120
 (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: emfy7xzXij35Su5kiAOInpq07Kb//1IITdpXNzM3gzK/dCZJLHBhyTR443elZ6EO
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL1PR12MB5127
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Mon, Oct 25, 2021 at 06:28:09AM +0000, Liu, Yi L wrote:
>    thanks for the guiding. will also refer to your vfio_group_cdev series.
> 
>    Need to double confirm here. Not quite following on the kfree. Is
>    this kfree to free the vfio_device structure? But now the
>    vfio_device pointer is provided by callers (e.g. vfio-pci). Do
>    you want to let vfio core allocate the vfio_device struct and
>    return the pointer to callers?

There are several common patterns for this problem, two that would be
suitable:

- Require each driver to provide a release op inside vfio_device_ops
  that does the kfree. Have the core provide a struct device release
  op that calls this one. Keep the kalloc/kfree in the drivers

- Move the kalloc into the core and have the core provide the kfree
  with an optional release callback for anydriver specific cleanup

  This requires some macro to make the memory layout work. RDMA has
  a version of this:

struct ib_device *_ib_alloc_device(size_t size);
#define ib_alloc_device(drv_struct, member)                                    \
        container_of(_ib_alloc_device(sizeof(struct drv_struct) +              \
                                      BUILD_BUG_ON_ZERO(offsetof(              \
                                              struct drv_struct, member))),    \
                     struct drv_struct, member)


In part the choice is how many drivers require a release callback
anyhow, if they all do then the first is easier to understand. If only
few or none do then the latter is less code in drivers, and never
exposes the driver to the tricky transition from alloc to refcount
cleanup.

Jason