On 5/3/21 11:56 AM, Logan Gunthorpe wrote:
...
IMHO, it is better to have all of the pci_*() functions dealing with pci_dev
instead of dev, but it is also true that this is a larger change, so I
won't press the point too hard right now.
As a general rule, I'd agree with you. However, it's not good to blindly
follow the rule when there might be good reasons to do it differently.
In this case, the caller doesn't have PCI devices. The nvme fabrics code
has a number of block devices and an RDMA device. It doesn't even know
if these devices are backed by PCI devices and it doesn't have a direct
path to obtain the pci_dev.
Each struct device, might be turned into a pci_dev using the static
function find_parent_pci_dev(). If any device is not a PCI device then
we reject the P2PDMA transaction as not supported. Pushing the
find_parent_pci_dev() logic into the callers is, IMO, just asking the
callers to replicate a bunch of logic it shouldn't even be aware of
creating messier code as a result.
I guess my main concern here is that there are these pci*() functions
that somehow want to pass around struct device. If a layer is carefully
named throughout with pci in the function names, then something is still
misaligned. This can happen over time, of course. But the really best
patchsets try to avoid or mitigate the effect, by keeping names and
functionality carefully aligned.
Anyway, I've bugged you enough, I should just wait and see what the next
round looks like, at this point. :)
thanks,
--
John Hubbard
NVIDIA
