On Wed, Feb 12, 2020 at 03:55:31PM +0800, Jason Wang wrote:> > The ida_simple_remove should probably be part of the class release > > function to make everything work right > > It looks to me bus instead of class is the correct abstraction here since > the devices share a set of programming interface but not the semantics.device_release() doesn't call the bus release? You have dev, type or class to choose from. Type is rarely used and doesn't seem to be used by vdpa, so class seems the right choice Jason
On 2020/2/12 ??8:51, Jason Gunthorpe wrote:> On Wed, Feb 12, 2020 at 03:55:31PM +0800, Jason Wang wrote: >>> The ida_simple_remove should probably be part of the class release >>> function to make everything work right >> It looks to me bus instead of class is the correct abstraction here since >> the devices share a set of programming interface but not the semantics. > device_release() doesn't call the bus release?What it did is: ??????? if (dev->release) ??????????????? dev->release(dev); ??????? else if (dev->type && dev->type->release) ??????????????? dev->type->release(dev); ??????? else if (dev->class && dev->class->dev_release) ??????????????? dev->class->dev_release(dev); ??????? else ??????????????? WARN(1, KERN_ERR "Device '%s' does not have a release() function, it is broken and must be fixed. See Documentation/kobject.txt.\n", ??????????????????????? dev_name(dev)); So it looks not.> You have dev, type or > class to choose from. Type is rarely used and doesn't seem to be used > by vdpa, so class seems the right choice > > JasonYes, but my understanding is class and bus are mutually exclusive. So we can't add a class to a device which is already attached on a bus. Thanks
On Thu, Feb 13, 2020 at 11:34:10AM +0800, Jason Wang wrote:> > You have dev, type or > > class to choose from. Type is rarely used and doesn't seem to be used > > by vdpa, so class seems the right choice > > > > Jason > > Yes, but my understanding is class and bus are mutually exclusive. So we > can't add a class to a device which is already attached on a bus.While I suppose there are variations, typically 'class' devices are user facing things and 'bus' devices are internal facing (ie like a PCI device) So why is this using a bus? VDPA is a user facing object, so the driver should create a class vhost_vdpa device directly, and that driver should live in the drivers/vhost/ directory. For the PCI VF case this driver would bind to a PCI device like everything else For our future SF/ADI cases the driver would bind to some SF/ADI/whatever device on a bus. I don't see a reason for VDPA to be creating busses.. Jason