On 01/17/2010 01:01 PM, Matthew Wilcox wrote:
> On Fri, Jan 15, 2010 at 10:24:51AM -0800, Jesse Barnes wrote:
>> Any update on this one Yinghai or Matthew? I agree with Matthew's
>> cleanup, the no dev check should probably be in scan_slot rather than
>> each potential callee.
>
> Here's what I came up with. I decided to put the check in both to be
> robust against the case where the ARI capability is incorrect and tells
> us about a device that doesn't exist.
>
> ---
>
> Make pci_scan_slot more robust
>
> Yinghai pointed out that the new pci_scan_slot() crashes when called
> on an ARI-capable slot that is empty. Fix this by exiting early from
> pci_scan_slot if there is no device in the slot.
>
> Also make next_ari_func() robust against devices not existing in case
> the ARI capability is corrupt. ARI also requires that the devices be
> listed in order, so if we find a function listed that is out of order,
> stop scanning to prevent loops.
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 11824d7..a06887f 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1222,11 +1222,19 @@ EXPORT_SYMBOL(pci_scan_single_device);
> static unsigned next_ari_fn(struct pci_dev *dev, unsigned fn)
> {
> u16 cap;
> - unsigned pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ARI);
> + unsigned pos, next_fn;
> +
> + if (!dev)
> + return 0;
this patch should work too because of checking here.
> +
> + pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ARI);
> if (!pos)
> return 0;
> pci_read_config_word(dev, pos + 4, &cap);
> - return cap >> 8;
> + next_fn = cap >> 8;
> + if (next_fn <= fn)
> + return 0;
> + return next_fn;
> }
>
> static unsigned next_trad_fn(struct pci_dev *dev, unsigned fn)
> @@ -1271,12 +1279,14 @@ int pci_scan_slot(struct pci_bus *bus, int devfn)
> return 0; /* Already scanned the entire slot */
>
> dev = pci_scan_single_device(bus, devfn);
> - if (dev && !dev->is_added) /* new device? */
> + if (!dev)
> + return 0;
> + if (!dev->is_added)
> nr++;
>
> if (pci_ari_enabled(bus))
> next_fn = next_ari_fn;
> - else if (dev && dev->multifunction)
> + else if (dev->multifunction)
> next_fn = next_trad_fn;
>
> for (fn = next_fn(dev, 0); fn > 0; fn = next_fn(dev, fn)) {
>
YH
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
