On Thu, 28 Jul 2016 14:52:59 +0000
Wei Yang <[email protected]> wrote:

> According to the comments and code, get_pci_function_alias_group() and
> get_pci_alias_group() do the search on the same pci bus. This means we can
> just iterate on the bus the pci device attaches to.
> 
> This patch narrows the search range by just iterating on the current bus
> and fix one typo in comment.
> 
> Signed-off-by: Wei Yang <[email protected]>
> ---
>  drivers/iommu/iommu.c |   22 ++++++++--------------
>  1 file changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 3000051..0338a81 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -643,17 +643,15 @@ static struct iommu_group 
> *get_pci_function_alias_group(struct pci_dev *pdev,
>       if (!pdev->multifunction || pci_acs_enabled(pdev, REQ_ACS_FLAGS))
>               return NULL;
>  
> -     for_each_pci_dev(tmp) {
> -             if (tmp == pdev || tmp->bus != pdev->bus ||
> +     list_for_each_entry(tmp, &pdev->bus->devices, bus_list) {

I believe this was using for_each_pci_dev() exactly because it takes a
reference to the device and therefore we can use it to get a reference
to the group.  How do you justify removing these reference semantics?
Thanks,

Alex

> +             if (tmp == pdev ||
>                   PCI_SLOT(tmp->devfn) != PCI_SLOT(pdev->devfn) ||
>                   pci_acs_enabled(tmp, REQ_ACS_FLAGS))
>                       continue;
>  
>               group = get_pci_alias_group(tmp, devfns);
> -             if (group) {
> -                     pci_dev_put(tmp);
> +             if (group)
>                       return group;
> -             }
>       }
>  
>       return NULL;
> @@ -681,23 +679,19 @@ static struct iommu_group *get_pci_alias_group(struct 
> pci_dev *pdev,
>       if (group)
>               return group;
>  
> -     for_each_pci_dev(tmp) {
> -             if (tmp == pdev || tmp->bus != pdev->bus)
> +     list_for_each_entry(tmp, &pdev->bus->devices, bus_list) {
> +             if (tmp == pdev)
>                       continue;
>  
>               /* We alias them or they alias us */
>               if (pci_devs_are_dma_aliases(pdev, tmp)) {
>                       group = get_pci_alias_group(tmp, devfns);
> -                     if (group) {
> -                             pci_dev_put(tmp);
> +                     if (group)
>                               return group;
> -                     }
>  
>                       group = get_pci_function_alias_group(tmp, devfns);
> -                     if (group) {
> -                             pci_dev_put(tmp);
> +                     if (group)
>                               return group;
> -                     }
>               }
>       }
>  
> @@ -794,7 +788,7 @@ struct iommu_group *pci_device_group(struct device *dev)
>  
>       /*
>        * Look for existing groups on non-isolated functions on the same
> -      * slot and aliases of those funcions, if any.  No need to clear
> +      * slot and aliases of those functions, if any. No need to clear
>        * the search bitmap, the tested devfns are still valid.
>        */
>       group = get_pci_function_alias_group(pdev, (unsigned long *)devfns);

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to