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. > > 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. > > > > 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.. Jason
