On Tue, 2024-10-15 at 10:02 -0300, Jason Gunthorpe wrote: > On Tue, Oct 15, 2024 at 02:41:24PM +0200, Thomas Hellström wrote: > > > It has nothing to do with kernel P2P, you are just allowing more > > > selective filtering of dev_private_owner. You should focus on > > > that in > > > the naming, not p2p. ie allow_dev_private() > > > > > > P2P is stuff that is dealing with MEMORY_DEVICE_PCI_P2PDMA. > > > > Yes, although the intention was to incorporate also other fast > > interconnects in "P2P", not just "PCIe P2P", but I'll definitely > > take a > > look at the naming. > > It has nothing to do with that, you are just filtering the device > private pages differently than default. > > Your end use might be P2P, but at this API level it certainly is not. Sure. Will find something more suitable. > > > > This is just allowing more instances of the same driver to co- > > > ordinate > > > their device private memory handle, for whatever purpose. > > > > Exactly, or theoretically even cross-driver. > > I don't want to see things like drivers changing their pgmap handles > privately somehow. If we are going to make it cross driver then it > needs to be generalized alot more. Cross-driver is initially not a thing, so let's worry about that later. My impression though is that this is the only change required for hmm_range_fault() and that infrastructure for opt-in and dma-mapping would need to be provided elsewhere? > > > > > > > Otherwise I don't see a particular problem, though we have talked > > > about widening the matching for device_private more broadly using > > > some > > > kind of grouping tag or something like that instead of a > > > callback. > > > You > > > may consider that as an alternative > > > > Yes. Looked at that, but (if I understand you correctly) that would > > be > > the case mentioned in the commit message where the group would be > > set > > up statically at dev_pagemap creation time? > > Not necessarily statically, but the membership would be stored in the > pagemap and by updated during hotplug/etc > > If this is for P2P then the dynamic behavior is pretty limited, some > kind of NxN bitmap. > > > > hmm_range struct inside a caller private data struct and use that > > > instead if inventing a whole new struct and pointer. > > > > Our first attempt was based on that but then that wouldn't be > > reusable > > in the migrate_device.c code. Hence the extra indirection. > > It is performance path, you should prefer duplication rather than > slowing it down.. OK. Will look at duplicating. Thanks, Thomas > > Jason
