Messages in this thread | | | Subject | Re: [PATCH 02/16] PCI/P2PDMA: Avoid pci_get_slot() which sleeps | From | Don Dutile <> | Date | Tue, 11 May 2021 12:05:34 -0400 |
| |
On 4/8/21 1:01 PM, Logan Gunthorpe wrote: > In order to use upstream_bridge_distance_warn() from a dma_map function, > it must not sleep. However, pci_get_slot() takes the pci_bus_sem so it > might sleep. > > In order to avoid this, try to get the host bridge's device from > bus->self, and if that is not set, just get the first element in the > device list. It should be impossible for the host bridge's device to > go away while references are held on child devices, so the first element > should not be able to change and, thus, this should be safe. Bjorn: Why wouldn't (shouldn't?) the bus->self field be set for a host bridge device? Should this situation be repaired in the host-brige config/setup code elsewhere in the kernel. ... and here, a check-and-fail with info of what doesn't have it setup (another new pci function to do the check & prinfo), so it can point to the offending host-bridge, and thus, the code that needs to be updated?
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com> > --- > drivers/pci/p2pdma.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c > index bd89437faf06..473a08940fbc 100644 > --- a/drivers/pci/p2pdma.c > +++ b/drivers/pci/p2pdma.c > @@ -311,16 +311,26 @@ static const struct pci_p2pdma_whitelist_entry { > static bool __host_bridge_whitelist(struct pci_host_bridge *host, > bool same_host_bridge) > { > - struct pci_dev *root = pci_get_slot(host->bus, PCI_DEVFN(0, 0)); > const struct pci_p2pdma_whitelist_entry *entry; > + struct pci_dev *root = host->bus->self; > unsigned short vendor, device; > > + /* > + * This makes the assumption that the first device on the bus is the > + * bridge itself and it has the devfn of 00.0. This assumption should > + * hold for the devices in the white list above, and if there are cases > + * where this isn't true they will have to be dealt with when such a > + * case is added to the whitelist. > + */ > if (!root) > + root = list_first_entry_or_null(&host->bus->devices, > + struct pci_dev, bus_list); > + > + if (!root || root->devfn) > return false; > > vendor = root->vendor; > device = root->device; > - pci_dev_put(root); > > for (entry = pci_p2pdma_whitelist; entry->vendor; entry++) { > if (vendor != entry->vendor || device != entry->device)
| |