Re: [PATCH 02/16] PCI/P2PDMA: Avoid pci_get_slot() which sleeps
From: Logan Gunthorpe
Date: Tue May 11 2021 - 12:16:54 EST
On 2021-05-11 10:05 a.m., Don Dutile wrote:
> ... add a flag (set for p2pdma use) to the function to print out what the root->devfn is, and what
> the device is so the needed quirk &/or modification can added to handle when this assumption fails;
> or make it a prdebug that can be flipped on for this failing situation, again, to add needed change to accomodate.
Good idea! Will add.
>> root = NULL;
>> out:
>> pci_dev_get(root);
>> return root;
>> }
>> EXPORT_SYMBOL(pci_get_root_slot);
>>
>> ...I think that's a lot clearer to the reader, about what's going on here.
>>
>> Note that I'm not really sure if it *is* safe, I would need to ask other
>> PCIe subsystem developers with more experience. But I don't think anyone
>> is trying to make p2pdma calls so early that PCIe buses are uninitialized.
>>
>>
>>> +
>>> + if (!root || root->devfn)
>>> return false;
>>> vendor = root->vendor;
>>> device = root->device;
>>> - pci_dev_put(root);
> and the reason to remove the dev_put is b/c it can sleep as well?
> is that ok, given the dev_get that John put into the new pci_get_root_slot()?
> ... seems like a locking version with no get/put's is needed, or, fix the host-bridge setups so no !NULL self pointers.
The dev_get is redundant here seeing we hold references to child
devices. It was only in the previous code because we were using
pci_get_slot() to get the device which did the get for us.
Logan