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