On 2022-07-05 11:42, Greg Kroah-Hartman wrote: > On Tue, Jul 05, 2022 at 11:32:23AM -0600, Logan Gunthorpe wrote: >> >> >> On 2022-07-05 11:21, Greg Kroah-Hartman wrote: >>> On Tue, Jul 05, 2022 at 06:50:39PM +0200, Christoph Hellwig wrote: >>>> [note for the newcomers, this is about allowing mmap()ing the PCIe >>>> P2P memory from the generic PCI P2P code through sysfs, and more >>>> importantly how to revoke it on device removal] >>> >>> We allow mmap on PCIe config space today, right? Why is this different >>> from what pci_create_legacy_files() does today? >>> >>>> On Tue, Jul 05, 2022 at 10:44:49AM -0600, Logan Gunthorpe wrote: >>>>> We might be able to. I'm not sure. I'll have to figure out how to find >>>>> that inode from the p2pdma code. I haven't found an obvious interface to >>>>> do that. >>>> >>>> I think the right way to approach this would be a new sysfs API >>>> that internally calls unmap_mapping_range internally instead of >>>> exposing the inode. I suspect that might actually be the right thing >>>> to do for iomem_inode as well. >>> >>> Why do we need something new and how is this any different from the PCI >>> binary files I mention above? We have supported PCI hotplug for a very >>> long time, do the current PCI binary sysfs files not work properly with >>> mmap and removing a device? >> >> The P2PDMA code allocates and hands out struct pages to userspace that >> are backed with ZONE_DEVICE memory from a device's BAR. This is quite >> different from the existing binary files mentioned above which neither >> support struct pages nor allocation. > > Why would you want to do this through a sysfs interface? that feels > horrid... The current version does it through a char device, but that requires creating a simple_fs and anon_inode for teardown on driver removal, plus a bunch of hooks through the driver that exposes it (NVMe, in this case) to set this all up. Christoph is suggesting a sysfs interface which could potentially avoid the anon_inode and all of the extra hooks. It has some significant benefits and maybe some small downsides, but I wouldn't describe it as horrid. Logan
