On Fri, Oct 30, 2020 at 04:30:45PM -0300, Jason Gunthorpe wrote: > On Fri, Oct 30, 2020 at 12:23:25PM -0700, Raj, Ashok wrote: > > On Fri, Oct 30, 2020 at 04:17:06PM -0300, Jason Gunthorpe wrote: > > > On Fri, Oct 30, 2020 at 12:13:48PM -0700, Dave Jiang wrote: > > > > > > > > > > > > On 10/30/2020 11:58 AM, Jason Gunthorpe wrote: > > > > > On Fri, Oct 30, 2020 at 11:50:47AM -0700, Dave Jiang wrote: > > > > > > .../ABI/stable/sysfs-driver-dma-idxd | 6 + > > > > > > Documentation/driver-api/vfio/mdev-idxd.rst | 404 ++++++ > > > > > > MAINTAINERS | 1 + > > > > > > drivers/dma/Kconfig | 9 + > > > > > > drivers/dma/idxd/Makefile | 2 + > > > > > > drivers/dma/idxd/cdev.c | 6 +- > > > > > > drivers/dma/idxd/device.c | 294 ++++- > > > > > > drivers/dma/idxd/idxd.h | 67 +- > > > > > > drivers/dma/idxd/init.c | 86 ++ > > > > > > drivers/dma/idxd/irq.c | 6 +- > > > > > > drivers/dma/idxd/mdev.c | 1121 +++++++++++++++++ > > > > > > drivers/dma/idxd/mdev.h | 116 ++ > > > > > > > > > > Again, a subsytem driver belongs in the directory hierarchy of the > > > > > subsystem, not in other random places. All this mdev stuff belongs > > > > > under drivers/vfio > > > > > > > > Alex seems to have disagreed last time.... > > > > https://lore.kernel.org/dmaengine/20200917113016.425dcde7@xxxxxxx/ > > > > > > Nobody else in the kernel is splitting subsystems up anymore > > > > > > > And I do agree with his perspective. The mdev is an extension of the PF > > > > driver. It's a bit awkward to be a stand alone mdev driver under vfio/mdev/. > > > > > > By this logic we'd have giagantic drivers under drivers/ethernet > > > touching netdev, rdma, scsi, vdpa, etc just because that is where the > > > PF driver came from. > > > > What makes you think this is providing services like scsi/rdma/vdpa etc.. ? > > > > for DSA this playes the exact same role, not a different function > > as you highlight above. these mdev's are creating DSA for virtualization > > use. They aren't providing a completely different role or subsystem per-se. > > It is a different subsystem, different maintainer, and different > reviewers. > > It is a development process problem, it doesn't matter what it is > doing. So drawing that parallel, do you expect all drivers that call pci_register_driver() to be located in drivers/pci? Aren't they scattered all over the place ata,scsi, platform drivers and such? As Alex pointed out, i915 and handful of s390 drivers that are mdev users are not in drivers/vfio. Are you sayint those drivers don't get reviewed? This is no different than PF driver offering VF services. Its a logical extension. Reviews happen for mdev users today. What you suggest seems like cutting the feet to fit the shoe. Unless the maintainers are asking things to be split just because its calling mdev_register_device() that practice doesn't exist and would be totally weird if you want to move all callers of pci_register_driver(). Your argument seems interesting even entertaining :-). But honestly i'm not finding it practical :-). So every caller of mmu_register_notifier() needs to be in mm? What you mention for different functions make absolute sense, not arguing against that. but this ain't that. And we just follow the asks of the maintainer. I know you aren't going to give up, but there is little we can do. I want the maintainers to make that call and I'm not add more noise to this. Cheers, Ashok
