Re: [PATCH 03/10] iommu: Add generic_single_device_group()

[Jason Gunthorpe <jgg@nvidia.com>]

On Fri, Jul 21, 2023 at 07:17:39AM +0000, Tian, Kevin wrote:
> > From: Jason Gunthorpe <jgg@nvidia.com>
> > Sent: Wednesday, July 19, 2023 3:06 AM
> > 
> > This implements the common pattern seen in drivers of a single
> > iommu_group for the entire iommu driver. Implement this in core code
> > so the drivers that want this can select it from their ops.
> 
> strictly speaking it's per-iommu-instance group. 😊

Sure

This implements the common pattern seen in drivers of a single iommu_group
for the entire iommu driver instance. Implement this in core code so the
drivers that want this can select it from their ops.


> > +struct iommu_group *generic_single_device_group(struct device *dev)
> > +{
> > +	struct iommu_device *iommu = dev->iommu->iommu_dev;
> > +
> > +	lockdep_assert_held(&dev_iommu_group_lock);
> > +
> > +	if (!iommu->singleton_group) {
> > +		struct iommu_group *group;
> > +
> > +		group = iommu_group_alloc();
> > +		if (IS_ERR(group))
> > +			return group;
> > +		iommu->singleton_group = group;
> > +	}
> > +	return iommu_group_ref_get(iommu->singleton_group);
> 
> 'singleton' is a bit confusing whether it means a single
> group per instance or a group having only a single device.
> 
> I don't know a better name, but probably just using
> iommu->group is not worse than the 'singleton_' prefix...

I definately want it to have a special name otherwise it can be pretty
confusing.

The fact it is in a function called "single device group" seems clear
enough

I revised the comment like this:

/*
 * Generic device_group call-back function. It just allocates one
 * iommu-group per iommu driver instance shared by every device
 * probed by that iommu driver.
 */

Jason